CommitLogViewer should not fetch commits in serial
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Jan 2018 20:27:56 +0000 (20:27 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Jan 2018 20:27:56 +0000 (20:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182207

Rubber-stamped by Chris Dumez.

Fetch both the commits in the range as well as the preceding commit at once instead of
fetching the preceding commit only after the commits in the range had been fetched.

* browser-tests/commit-log-viewer-tests.js: Fixed the tcoest case after r224227.
* public/v3/components/commit-log-viewer.js:
(CommitLogViewer.prototype._fetchCommitLogs): Fetch commits in parallel.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227746 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/browser-tests/commit-log-viewer-tests.js
Websites/perf.webkit.org/public/v3/components/commit-log-viewer.js

index 0dffc27..5ed0339 100644 (file)
@@ -1,3 +1,17 @@
+2018-01-29  Ryosuke Niwa  <rniwa@webkit.org>
+
+        CommitLogViewer should not fetch commits in serial
+        https://bugs.webkit.org/show_bug.cgi?id=182207
+
+        Rubber-stamped by Chris Dumez.
+
+        Fetch both the commits in the range as well as the preceding commit at once instead of
+        fetching the preceding commit only after the commits in the range had been fetched.
+
+        * browser-tests/commit-log-viewer-tests.js: Fixed the tcoest case after r224227.
+        * public/v3/components/commit-log-viewer.js:
+        (CommitLogViewer.prototype._fetchCommitLogs): Fetch commits in parallel.
+
 2018-01-24  Dewei Zhu  <dewei_zhu@apple.com>
 
         Check existence of 'node_modules_dir' before creating it.
index 0c6b341..ea95688 100644 (file)
@@ -18,6 +18,18 @@ describe('CommitLogViewer', () => {
         });
     }
 
+    const webkitCommit210948 = {
+        "id": "185326",
+        "revision": "210948",
+        "repository": 1,
+        "previousCommit": null,
+        "ownsCommits": false,
+        "time": +new Date("2017-01-20 02:52:34.577Z"),
+        "authorName": "Zalan Bujtas",
+        "authorEmail": "zalan@apple.com",
+        "message": "a message",
+    };
+
     const webkitCommit210949 = {
         "id": "185334",
         "revision": "210949",
@@ -86,7 +98,7 @@ describe('CommitLogViewer', () => {
 
     it('should show the repository name, the list of commits, and hide the spinner once the list of commits are fetched', () => {
         const context = new BrowsingContext();
-        return importCommitLogViewer(context).then((CommitLogViewer) => {
+        return importCommitLogViewer(context).then(async (CommitLogViewer) => {
             const Repository = context.symbols.Repository;
             const SpinnerIcon = context.symbols.SpinnerIcon;
             const ComponentBase = context.symbols.ComponentBase;
@@ -94,33 +106,40 @@ describe('CommitLogViewer', () => {
             const viewer = new CommitLogViewer;
             const webkit = new Repository(1, {name: 'WebKit'});
             context.document.body.appendChild(viewer.element());
+
             viewer.enqueueToRender();
-            return waitForComponentsToRender(context).then(() => {
-                viewer.view(webkit, '210948', '210950');
-                return waitForComponentsToRender(context);
-            }).then(() => {
-                expect(viewer.content('spinner-container').offsetHeight).to.not.be(0);
-                expect(viewer.content('commits-list').matches(':empty')).to.be(true);
-                expect(viewer.content('repository-name').matches(':empty')).to.be(false);
-                expect(viewer.content('repository-name').textContent).to.contain('WebKit');
-                expect(requests.length).to.be(1);
-                expect(requests[0].url).to.be('/api/commits/1/?precedingRevision=210948&lastRevision=210950');
-                requests[0].resolve({
-                    "status": "OK",
-                    "commits": [webkitCommit210949, webkitCommit210950],
-                });
-                return waitForComponentsToRender(context);
-            }).then(() => {
-                expect(viewer.content('spinner-container').offsetHeight).to.be(0);
-                expect(viewer.content('commits-list').matches(':empty')).to.be(false);
-                expect(viewer.content('commits-list').textContent).to.contain('r210949');
-                expect(viewer.content('commits-list').textContent).to.contain('Chris Dumez');
-                expect(viewer.content('commits-list').textContent).to.contain('r210950');
-                expect(viewer.content('commits-list').textContent).to.contain('Commit Queue');
-                expect(viewer.content('repository-name').matches(':empty')).to.be(false);
-                expect(viewer.content('repository-name').textContent).to.contain('WebKit');
-                expect(viewer.content('commits-list').querySelector('a')).to.be(null);
+            await waitForComponentsToRender(context);
+
+            viewer.view(webkit, '210948', '210950');
+            await waitForComponentsToRender(context);
+
+            expect(viewer.content('spinner-container').offsetHeight).to.not.be(0);
+            expect(viewer.content('commits-list').matches(':empty')).to.be(true);
+            expect(viewer.content('repository-name').matches(':empty')).to.be(false);
+            expect(viewer.content('repository-name').textContent).to.contain('WebKit');
+            expect(requests.length).to.be(2);
+            expect(requests[0].url).to.be('/api/commits/1/?precedingRevision=210948&lastRevision=210950');
+            expect(requests[1].url).to.be('/api/commits/1/210948');
+            requests[0].resolve({
+                "status": "OK",
+                "commits": [webkitCommit210949, webkitCommit210950],
             });
+            requests[1].resolve({
+                "status": "OK",
+                "commits": [webkitCommit210948],
+            });
+
+            await waitForComponentsToRender(context);
+
+            expect(viewer.content('spinner-container').offsetHeight).to.be(0);
+            expect(viewer.content('commits-list').matches(':empty')).to.be(false);
+            expect(viewer.content('commits-list').textContent).to.contain('r210949');
+            expect(viewer.content('commits-list').textContent).to.contain('Chris Dumez');
+            expect(viewer.content('commits-list').textContent).to.contain('r210950');
+            expect(viewer.content('commits-list').textContent).to.contain('Commit Queue');
+            expect(viewer.content('repository-name').matches(':empty')).to.be(false);
+            expect(viewer.content('repository-name').textContent).to.contain('WebKit');
+            expect(viewer.content('commits-list').querySelector('a')).to.be(null);
         });
     });
 
index cc20c26..005b552 100644 (file)
@@ -36,11 +36,29 @@ class CommitLogViewer extends ComponentBase {
         }
 
         let promise;
+        let precedingCommitPromise;
         const fetchSingleCommit = !precedingRevision || precedingRevision == lastRevision;
-        if (fetchSingleCommit)
-            promise = CommitLog.fetchForSingleRevision(repository, lastRevision);
-        else
-            promise = CommitLog.fetchBetweenRevisions(repository, precedingRevision, lastRevision);
+        if (fetchSingleCommit) {
+            promise = CommitLog.fetchForSingleRevision(repository, lastRevision).then((commits) => {
+                if (this._fetchingPromise != promise)
+                    return;
+                this._commits = commits;
+                this._precedingCommit = null;
+            });
+        } else {
+            promise = Promise.all([
+                CommitLog.fetchBetweenRevisions(repository, precedingRevision, lastRevision).then((commits) => {
+                    if (this._fetchingPromise != promise)
+                        return;
+                    this._commits = commits;
+                }),
+                CommitLog.fetchForSingleRevision(repository, precedingRevision).then((precedingCommit) => {
+                    if (this._fetchingPromise != promise)
+                        return;
+                    this._precedingCommit = null;
+                })
+            ]);
+        }
 
         this._repository = repository;
         this._fetchingPromise = promise;
@@ -48,31 +66,14 @@ class CommitLogViewer extends ComponentBase {
         this._fetchingPromise.then((commits) => {
             if (this._fetchingPromise != promise)
                 return;
-            this._commits = commits;
-            if (fetchSingleCommit) {
-                this._fetchingPromise = null;
-                this._precedingCommit = null;
-                this.enqueueToRender();
-                return;
-            }
-            return CommitLog.fetchForSingleRevision(repository, precedingRevision).then((precedingCommit) => {
-                if (this._fetchingPromise != promise)
-                    return;
-                this._fetchingPromise = null;
-                this._precedingCommit = precedingCommit[0];
-                this.enqueueToRender();
-            }, (error) => {
-                if (this._fetchingPromise != promise)
-                    return;
-                this._fetchingPromise = null;
-                this._precedingCommit = null;
-                this.enqueueToRender();
-            });
+            this._fetchingPromise = null;
+            this.enqueueToRender();
         }, (error) => {
             if (this._fetchingPromise != promise)
                 return;
             this._fetchingPromise = null;
             this._commits = null;
+            this._precedingCommit = null;
             this.enqueueToRender();
         });