Merge "DiffViewDisplayed metric changed to view usable to start reviewing"
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.
