DiffViewDisplayed metric changed to view usable to start reviewing
This introduce DiffViewFullyLoaded and changing DiffViewDisplayed to
same convetion as ChangeViewDisplayed and ChangeViewFullyLoaded. This
also introduce DiffViewContentDisplayed which is event when content
displayed but we dont wait for a highlighting layer.
DiffViewDisplayed ends once we render 120 lines in diff, which is more
than full window viewport. DiffViewFullyLoaded ends when every line is
loaded same as before DiffViewDisplayed.
Change-Id: I99a0d44950054316305d4166e89b4e4f12eeb7a9
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
index 9ff425c..a0af7f4 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
@@ -70,13 +70,17 @@
CHANGE_DISPLAYED: 'ChangeDisplayed',
CHANGE_LOAD_FULL: 'ChangeFullyLoaded',
DASHBOARD_DISPLAYED: 'DashboardDisplayed',
+ DIFF_VIEW_CONTENT_DISPLAYED: 'DiffViewOnlyContent',
DIFF_VIEW_DISPLAYED: 'DiffViewDisplayed',
+ DIFF_VIEW_LOAD_FULL: 'DiffViewFullyLoaded',
FILE_LIST_DISPLAYED: 'FileListDisplayed',
PLUGINS_LOADED: 'PluginsLoaded',
STARTUP_CHANGE_DISPLAYED: 'StartupChangeDisplayed',
STARTUP_CHANGE_LOAD_FULL: 'StartupChangeFullyLoaded',
STARTUP_DASHBOARD_DISPLAYED: 'StartupDashboardDisplayed',
+ STARTUP_DIFF_VIEW_CONTENT_DISPLAYED: 'StartupDiffViewOnlyContent',
STARTUP_DIFF_VIEW_DISPLAYED: 'StartupDiffViewDisplayed',
+ STARTUP_DIFF_VIEW_LOAD_FULL: 'StartupDiffViewFullyLoaded',
STARTUP_FILE_LIST_DISPLAYED: 'StartupFileListDisplayed',
WEB_COMPONENTS_READY: 'WebComponentsReady',
METRICS_PLUGIN_LOADED: 'MetricsPluginLoaded',
@@ -88,7 +92,9 @@
STARTUP_TIMERS[TIMER.STARTUP_CHANGE_DISPLAYED] = 0;
STARTUP_TIMERS[TIMER.STARTUP_CHANGE_LOAD_FULL] = 0;
STARTUP_TIMERS[TIMER.STARTUP_DASHBOARD_DISPLAYED] = 0;
+ STARTUP_TIMERS[TIMER.STARTUP_DIFF_VIEW_CONTENT_DISPLAYED] = 0;
STARTUP_TIMERS[TIMER.STARTUP_DIFF_VIEW_DISPLAYED] = 0;
+ STARTUP_TIMERS[TIMER.STARTUP_DIFF_VIEW_LOAD_FULL] = 0;
STARTUP_TIMERS[TIMER.STARTUP_FILE_LIST_DISPLAYED] = 0;
STARTUP_TIMERS[TIMING.APP_STARTED] = 0;
// WebComponentsReady timer is triggered from gr-router.
@@ -287,7 +293,9 @@
this.time(TIMER.CHANGE_DISPLAYED);
this.time(TIMER.CHANGE_LOAD_FULL);
this.time(TIMER.DASHBOARD_DISPLAYED);
+ this.time(TIMER.DIFF_VIEW_CONTENT_DISPLAYED);
this.time(TIMER.DIFF_VIEW_DISPLAYED);
+ this.time(TIMER.DIFF_VIEW_LOAD_FULL);
this.time(TIMER.FILE_LIST_DISPLAYED);
},
@@ -328,6 +336,23 @@
}
},
+ diffViewFullyLoaded() {
+ if (this._baselines.hasOwnProperty(TIMER.STARTUP_DIFF_VIEW_LOAD_FULL)) {
+ this.timeEnd(TIMER.STARTUP_DIFF_VIEW_LOAD_FULL);
+ } else {
+ this.timeEnd(TIMER.DIFF_VIEW_LOAD_FULL);
+ }
+ },
+
+ diffViewContentDisplayed() {
+ if (this._baselines.hasOwnProperty(
+ TIMER.STARTUP_DIFF_VIEW_CONTENT_DISPLAYED)) {
+ this.timeEnd(TIMER.STARTUP_DIFF_VIEW_CONTENT_DISPLAYED);
+ } else {
+ this.timeEnd(TIMER.DIFF_VIEW_CONTENT_DISPLAYED);
+ }
+ },
+
fileListDisplayed() {
if (this._baselines.hasOwnProperty(TIMER.STARTUP_FILE_LIST_DISPLAYED)) {
this.timeEnd(TIMER.STARTUP_FILE_LIST_DISPLAYED);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 2284333..7382af6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -42,6 +42,9 @@
// syntax highlighting for the entire file.
const SYNTAX_MAX_LINE_LENGTH = 500;
+ // 120 lines is good enough threshold for full-sized window viewport
+ const NUM_OF_LINES_THRESHOLD_FOR_VIEWPORT = 120;
+
const WHITESPACE_IGNORE_NONE = 'IGNORE_NONE';
/**
@@ -269,8 +272,12 @@
});
},
- /** @return {!Promise} */
- reload() {
+ /**
+ * @param {boolean=} haveParamsChanged ends reporting events that started
+ * on location change.
+ * @return {!Promise}
+ **/
+ reload(haveParamsChanged) {
this._loading = true;
this._errorMessage = null;
const whitespaceLevel = this._getIgnoreWhitespace();
@@ -283,6 +290,11 @@
}
this._layers = layers;
+ if (haveParamsChanged) {
+ // We listen on render viewport only on DiffPage (on paramsChanged)
+ this._listenToViewportRender();
+ }
+
this._coverageRanges = [];
const {changeNum, path, patchRange: {basePatchNum, patchNum}} = this;
this.$.jsAPI.getCoverageRanges(changeNum, path, basePatchNum, patchNum).
@@ -900,6 +912,17 @@
});
},
+ _listenToViewportRender() {
+ const renderUpdateListener = start => {
+ if (start > NUM_OF_LINES_THRESHOLD_FOR_VIEWPORT) {
+ this.$.reporting.diffViewDisplayed();
+ this.$.syntaxLayer.removeListener(renderUpdateListener);
+ }
+ };
+
+ this.$.syntaxLayer.addListener(renderUpdateListener);
+ },
+
_handleRenderStart() {
this.$.reporting.time(TimingLabel.TOTAL);
this.$.reporting.time(TimingLabel.CONTENT);
@@ -907,6 +930,7 @@
_handleRenderContent() {
this.$.reporting.timeEnd(TimingLabel.CONTENT);
+ this.$.reporting.diffViewContentDisplayed();
},
_handleNormalizeRange(event) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index 58edfdb..cdbdeb5 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -327,11 +327,12 @@
notifySyntaxProcessed();
// Assert after the notification task is processed.
Promise.resolve().then(() => {
- assert.isTrue(element.$.reporting.timeEnd.calledThrice);
assert.isTrue(element.$.reporting.timeEnd.calledWithExactly(
'Diff Total Render'));
assert.isTrue(element.$.reporting.timeEnd.calledWithExactly(
'Diff Syntax Render'));
+ assert.isTrue(element.$.reporting.timeEnd.calledWithExactly(
+ 'StartupDiffViewOnlyContent'));
done();
});
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index eb298c8..b2ba148 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -674,8 +674,10 @@
}
this._loading = false;
this.$.diffHost.comments = this._commentsForDiff;
- return this.$.diffHost.reload();
+ return this.$.diffHost.reload(true);
}).then(() => {
+ this.$.reporting.diffViewFullyLoaded();
+ // If diff view displayed has not ended yet, it ends here.
this.$.reporting.diffViewDisplayed();
});
},
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
index 28eea44..383687f 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
@@ -174,6 +174,10 @@
this.push('_listeners', fn);
},
+ removeListener(fn) {
+ this._listeners = this._listeners.filter(f => f != fn);
+ },
+
/**
* Annotation layer method to add syntax annotations to the given element
* for the given line.
