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
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -327,11 +327,12 @@ limitations under the License.
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
},
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user