Merge changes from topic "issue8324"
* changes:
Update _fetchSharedCacheURL to accept Defs.FetchJSONRequest
Refactor optional _getChangeURLAndFetch arguments to objects
Refactor optional JSON fetch arguments to objects
Make _fetchJSON method private
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index b59742e..3f26628 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -283,6 +283,7 @@
as="file"
initial-count="[[fileListIncrement]]"
target-framerate="1">
+ [[_reportRenderedRow(index)]]
<div class="stickyArea">
<div class$="file-row row [[_computePathClass(file.__path, _expandedFilePaths.*)]]"
data-path$="[[file.__path]]" tabindex="-1">
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 0fa037c..c1eb39c 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -26,6 +26,8 @@
const SIZE_BAR_GAP_WIDTH = 1;
const SIZE_BAR_MIN_WIDTH = 1.5;
+ const RENDER_TIME = 'FileListRenderTime';
+
const FileStatus = {
A: 'Added',
C: 'Copied',
@@ -797,6 +799,12 @@
_computeFilesShown(numFilesShown, files) {
const filesShown = files.base.slice(0, numFilesShown);
this.fire('files-shown-changed', {length: filesShown.length});
+
+ // Start the timer for the rendering work hwere because this is where the
+ // _shownFiles property is being set, and _shownFiles is used in the
+ // dom-repeat binding.
+ this.$.reporting.time(RENDER_TIME);
+
return filesShown;
},
@@ -1175,5 +1183,21 @@
_noDiffsExpanded() {
return this.filesExpanded === GrFileListConstants.FilesExpandedState.NONE;
},
+
+ /**
+ * Method to call via binding when each file list row is rendered. This
+ * allows approximate detection of when the dom-repeat has completed
+ * rendering.
+ * @param {number} index The index of the row being rendered.
+ * @return {string} an empty string.
+ */
+ _reportRenderedRow(index) {
+ if (index === this._shownFiles.length - 1) {
+ this.async(() => {
+ this.$.reporting.timeEnd(RENDER_TIME);
+ }, 1);
+ }
+ return '';
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index f303da7..ea95613 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -126,6 +126,19 @@
assert.isTrue(controlRow.classList.contains('invisible'));
});
+ test('rendering each row calls the _reportRenderedRow method', () => {
+ const renderedStub = sandbox.stub(element, '_reportRenderedRow');
+ element._filesByPath = _.range(10)
+ .reduce((_filesByPath, i) => {
+ _filesByPath['/file' + i] = {lines_inserted: 9};
+ return _filesByPath;
+ }, {});
+ flushAsynchronousOperations();
+ assert.equal(
+ Polymer.dom(element.root).querySelectorAll('.file-row').length, 10);
+ assert.equal(renderedStub.callCount, 10);
+ });
+
test('calculate totals for patch number', () => {
element._filesByPath = {
'/COMMIT_MSG': {
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector.js
new file mode 100644
index 0000000..28c46f4
--- /dev/null
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector.js
@@ -0,0 +1,61 @@
+/**
+ * @license
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+(function() {
+ 'use strict';
+
+ const JANK_SLEEP_TIME_MS = 1000;
+
+ const GrJankDetector = {
+ // Slowdowns counter.
+ jank: 0,
+ fps: 0,
+ _lastFrameTime: 0,
+
+ start() {
+ this._requestAnimationFrame(this._detect.bind(this));
+ },
+
+ _requestAnimationFrame(callback) {
+ window.requestAnimationFrame(callback);
+ },
+
+ _detect(now) {
+ if (this._lastFrameTime === 0) {
+ this._lastFrameTime = now;
+ this.fps = 0;
+ this._requestAnimationFrame(this._detect.bind(this));
+ return;
+ }
+ const fpsNow = 1000/(now - this._lastFrameTime);
+ this._lastFrameTime = now;
+ // Calculate moving average within last 3 measurements.
+ this.fps = this.fps === 0 ? fpsNow : ((this.fps * 2 + fpsNow) / 3);
+ if (this.fps > 10) {
+ this._requestAnimationFrame(this._detect.bind(this));
+ } else {
+ this.jank++;
+ console.warn('JANK', this.jank);
+ this._lastFrameTime = 0;
+ window.setTimeout(
+ () => this._requestAnimationFrame(this._detect.bind(this)),
+ JANK_SLEEP_TIME_MS);
+ }
+ },
+ };
+
+ window.GrJankDetector = GrJankDetector;
+})();
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector_test.html b/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector_test.html
new file mode 100644
index 0000000..6faeec1
--- /dev/null
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector_test.html
@@ -0,0 +1,78 @@
+<!DOCTYPE html>
+<!--
+@license
+Copyright (C) 2018 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-jank-detector</title>
+
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../../test/common-test-setup.html"/>
+
+<script src="gr-jank-detector.js"></script>
+
+<script>
+ suite('gr-jank-detector tests', () => {
+ let sandbox;
+ let clock;
+ let instance;
+
+ const NOW_TIME = 100;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ clock = sinon.useFakeTimers(NOW_TIME);
+ instance = GrJankDetector;
+ instance._lastFrameTime = 0;
+ sandbox.stub(instance, '_requestAnimationFrame');
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ test('start() installs frame callback', () => {
+ sandbox.stub(instance, '_detect');
+ instance._requestAnimationFrame.callsArg(0);
+ instance.start();
+ assert.isTrue(instance._detect.calledOnce);
+ });
+
+ test('measures fps', () => {
+ instance._detect(10);
+ instance._detect(30);
+ assert.equal(instance.fps, 50);
+ });
+
+ test('detects jank', () => {
+ let now = 10;
+ instance._detect(now);
+ const fastFrame = () => instance._detect(now += 20);
+ const slowFrame = () => instance._detect(now += 300);
+ fastFrame();
+ assert.equal(instance.jank, 0);
+ _.times(4, slowFrame);
+ assert.equal(instance.jank, 0);
+ instance._requestAnimationFrame.reset();
+ slowFrame();
+ assert.equal(instance.jank, 1);
+ assert.isFalse(instance._requestAnimationFrame.called);
+ clock.tick(1000);
+ assert.isTrue(instance._requestAnimationFrame.called);
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.html b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.html
index 2970a26..cbb2c09 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.html
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.html
@@ -19,5 +19,6 @@
<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<dom-module id="gr-reporting">
+ <script src="gr-jank-detector.js"></script>
<script src="gr-reporting.js"></script>
</dom-module>
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 0db442f..ae67dac 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
@@ -48,6 +48,14 @@
STARTED_HIDDEN: 'hidden',
};
+ // Frame rate related constants.
+ const JANK = {
+ TYPE: 'lifecycle',
+ CATEGORY: 'UI Latency',
+ // Reported events - alphabetize below.
+ COUNT: 'Jank count',
+ };
+
// Navigation reporting constants.
const NAVIGATION = {
TYPE: 'nav-report',
@@ -118,6 +126,8 @@
};
catchErrors();
+ GrJankDetector.start();
+
const GrReporting = Polymer({
is: 'gr-reporting',
@@ -206,6 +216,11 @@
},
beforeLocationChanged() {
+ if (GrJankDetector.jank > 0) {
+ this.reporter(
+ JANK.TYPE, JANK.CATEGORY, JANK.COUNT, GrJankDetector.jank);
+ GrJankDetector.jank = 0;
+ }
for (const prop of Object.keys(this._baselines)) {
delete this._baselines[prop];
}
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
index bfb45f6..e2bb83d 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
@@ -93,7 +93,11 @@
test('beforeLocationChanged', () => {
element._baselines['garbage'] = 'monster';
sandbox.stub(element, 'time');
+ GrJankDetector.jank = 42;
element.beforeLocationChanged();
+ assert.equal(GrJankDetector.jank, 0);
+ assert.isTrue(element.reporter.calledWithExactly(
+ 'lifecycle', 'UI Latency', 'Jank count', 42));
assert.isTrue(element.time.calledWithExactly('DashboardDisplayed'));
assert.isTrue(element.time.calledWithExactly('ChangeDisplayed'));
assert.isTrue(element.time.calledWithExactly('DiffViewDisplayed'));
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 6cf674a..6a562fc 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -88,6 +88,7 @@
'core/gr-error-manager/gr-error-manager_test.html',
'core/gr-main-header/gr-main-header_test.html',
'core/gr-navigation/gr-navigation_test.html',
+ 'core/gr-reporting/gr-jank-detector_test.html',
'core/gr-reporting/gr-reporting_test.html',
'core/gr-router/gr-router_test.html',
'core/gr-search-bar/gr-search-bar_test.html',
