From ba69a1e19231e0fe15dac684a8578cd51a47a1c6 Mon Sep 17 00:00:00 2001
From: Viktar Donich <viktard@google.com>
Date: Tue, 1 Nov 2016 14:12:07 -0700
Subject: [PATCH] Coalesce requests within change view

Fetch more data with GET /change/detail instead of multiple separate
GET requests. Fetch independent resources in parallel. Fixed change
actions jumping around.

First page load to full page down by ~25% (1050ms vs 1400ms on localhost,
expect even more on prod server)

Timeline before and after:
http://imgur.com/a/oEFdD

Bug: Issue 4406
Change-Id: Ib7c3c160513cbb29aad89fedabfacbfd271abf22
---
 .../gr-change-actions/gr-change-actions.html  |  2 +-
 .../gr-change-actions/gr-change-actions.js    | 20 +++++-----
 .../change/gr-change-view/gr-change-view.html |  1 +
 .../change/gr-change-view/gr-change-view.js   | 40 +++++++++----------
 .../gr-change-view/gr-change-view_test.html   |  9 ++++-
 .../gr-rest-api-interface.html                |  1 -
 .../gr-rest-api-interface.js                  |  6 ++-
 7 files changed, 41 insertions(+), 38 deletions(-)

diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index 30e9e86676..bd96c2f2cc 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -76,7 +76,7 @@ limitations under the License.
               on-tap="_handleActionTap"></gr-button>
         </template>
       </section>
-      <section hidden$="[[!_actionCount(_revisionActions.*, _additionalActions.*)]]">
+      <section hidden$="[[!_actionCount(revisionActions.*, _additionalActions.*)]]">
         <template is="dom-repeat" items="[[_revisionActionValues]]" as="action">
           <gr-button title$="[[action.title]]"
               hidden$="[[_computeActionHidden(action.__key, _hiddenRevisionActions.*)]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index ef2a3b4456..357858afa8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -79,18 +79,18 @@
         type: String,
         value: '',
       },
+      revisionActions: {
+        type: Object,
+        value: function() { return {}; },
+      },
 
       _loading: {
         type: Boolean,
         value: true,
       },
-      _revisionActions: {
-        type: Object,
-        value: function() { return {}; },
-      },
       _revisionActionValues: {
         type: Array,
-        computed: '_computeRevisionActionValues(_revisionActions.*, ' +
+        computed: '_computeRevisionActionValues(revisionActions.*, ' +
             'primaryActionKeys.*, _additionalActions.*)',
       },
       _changeActionValues: {
@@ -121,7 +121,7 @@
     ],
 
     observers: [
-      '_actionsChanged(actions.*, _revisionActions.*, _additionalActions.*)',
+      '_actionsChanged(actions.*, revisionActions.*, _additionalActions.*)',
     ],
 
     ready: function() {
@@ -137,7 +137,7 @@
       return this._getRevisionActions().then(function(revisionActions) {
         if (!revisionActions) { return; }
 
-        this._revisionActions = revisionActions;
+        this.revisionActions = revisionActions;
         this._loading = false;
       }.bind(this)).catch(function(err) {
         alert('Couldn’t load revision actions. Check the console ' +
@@ -363,7 +363,7 @@
           /* falls through */ // required by JSHint
         default:
           this._fireAction(this._prependSlash(key),
-              this._revisionActions[key], true);
+              this.revisionActions[key], true);
       }
     },
 
@@ -400,7 +400,7 @@
       }
       this.$.overlay.close();
       el.hidden = true;
-      this._fireAction('/rebase', this._revisionActions.rebase, true, payload);
+      this._fireAction('/rebase', this.revisionActions.rebase, true, payload);
     },
 
     _handleCherrypickConfirm: function() {
@@ -418,7 +418,7 @@
       el.hidden = true;
       this._fireAction(
           '/cherrypick',
-          this._revisionActions.cherrypick,
+          this.revisionActions.cherrypick,
           true,
           {
             destination: el.branch,
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 00ebd4957b..17f96c1431 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -248,6 +248,7 @@ limitations under the License.
             <gr-change-actions id="actions"
                 change="[[_change]]"
                 actions="[[_change.actions]]"
+                revision-actions="[[_currentRevisionActions]]"
                 change-num="[[_changeNum]]"
                 patch-num="[[_computeLatestPatchNum(_allPatchSets)]]"
                 commit-message="[[_latestCommitMessage]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index bbd4d2df55..a41e30dfcb 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -55,6 +55,7 @@
         observer: '_changeChanged',
       },
       _commitInfo: Object,
+      _files: Object,
       _changeNum: String,
       _diffDrafts: {
         type: Object,
@@ -77,6 +78,7 @@
         type: Object,
         observer: '_updateSelected',
       },
+      _currentRevisionActions: Object,
       _allPatchSets: {
         type: Array,
         computed: '_computeAllPatchSets(_change)',
@@ -681,7 +683,16 @@
                 if (!change.reviewer_updates) {
                   change.reviewer_updates = null;
                 }
+                var currentRevision = change.revisions &&
+                    change.revisions[change.current_revision];
+                this._latestCommitMessage = currentRevision.commit.message;
                 this._change = change;
+                if (!this._patchRange || !this._patchRange.patchNum ||
+                    this._patchRange.patchNum === currentRevision._number) {
+                  this._commitInfo = currentRevision.commit;
+                  this._currentRevisionActions = currentRevision.actions;
+                  // TODO: Fetch and process files.
+                }
               }.bind(this));
     },
 
@@ -728,40 +739,25 @@
 
       var detailCompletes = this._getChangeDetail().then(function() {
         this._loading = false;
+        this._getProjectConfig();
       }.bind(this));
       this._getComments();
 
       if (this._patchRange.patchNum) {
-        return this._reloadPatchNumDependentResources().then(function() {
-          return detailCompletes;
-        }).then(function() {
-          return this._reloadDetailDependentResources();
+        return Promise.all([
+          this._reloadPatchNumDependentResources(),
+          detailCompletes,
+        ]).then(function() {
+          return this.$.actions.reload();
         }.bind(this));
       } else {
         // The patch number is reliant on the change detail request.
         return detailCompletes.then(function() {
-          return this._reloadPatchNumDependentResources();
-        }.bind(this)).then(function() {
-          return this._reloadDetailDependentResources();
+          this.$.fileList.reload();
         }.bind(this));
       }
     },
 
-    /**
-     * Kicks off requests for resources that rely on the change detail
-     * (`this._change`) being loaded.
-     */
-    _reloadDetailDependentResources: function() {
-      if (!this._change) { return Promise.resolve(); }
-
-      return this._getProjectConfig().then(function() {
-        return Promise.all([
-          this._getLatestCommitMessage(),
-          this.$.actions.reload(),
-        ]);
-      }.bind(this));
-    },
-
     /**
      * Kicks off requests for resources that rely on the patch range
      * (`this._patchRange`) being defined.
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 7c8a329aed..51b4453dd1 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -102,7 +102,7 @@ limitations under the License.
           return Promise.resolve({
             change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
             revisions: {
-              rev1: {_number: 1},
+              rev1: {_number: 1, commit: {}},
               rev13: {_number: 13},
             },
             current_revision: 'rev1',
@@ -468,7 +468,12 @@ limitations under the License.
     test('topic is coalesced to null', function(done) {
       sandbox.stub(element, '_changeChanged');
       sandbox.stub(element.$.restAPI, 'getChangeDetail', function() {
-        return Promise.resolve({id: '123456789', labels: {}});
+        return Promise.resolve({
+          id: '123456789',
+          labels: {},
+          current_revision: 'foo',
+          revisions: {foo: {commit: {}}},
+        });
       });
 
       element._getChangeDetail().then(function() {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
index f34ffcf32c..b5c9f0c94d 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
@@ -22,4 +22,3 @@ limitations under the License.
 <dom-module id="gr-rest-api-interface">
   <script src="gr-rest-api-interface.js"></script>
 </dom-module>
-
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 96a8bcadd8..07ccf2b899 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -388,11 +388,13 @@
       var options = this._listChangesOptionsToHex(
           ListChangesOption.ALL_REVISIONS,
           ListChangesOption.CHANGE_ACTIONS,
+          ListChangesOption.CURRENT_ACTIONS,
+          ListChangesOption.CURRENT_COMMIT,
           ListChangesOption.DOWNLOAD_COMMANDS,
           ListChangesOption.SUBMITTABLE
       );
-      return this._getChangeDetail(changeNum, options, opt_errFn,
-          opt_cancelCondition);
+      return this._getChangeDetail(
+          changeNum, options, opt_errFn, opt_cancelCondition);
     },
 
     getDiffChangeDetail: function(changeNum, opt_errFn, opt_cancelCondition) {