Merge changes I04602bec,I8a25c006,Id6f8114f,I5a1d639e,Ieca03602

* changes:
  Make reinitializing cursor explicit
  Move temp scrolling overwrite into cursor init
  Reset scroll listening closer to where its set
  Clarify why we listen to scrolling
  Update tab stops only once when diff changes

polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js

@@ -275,40 +275,44 @@ class GrDiffCursor extends mixinBehaviors([Gerrit.FireBehavior],
   }
 
   reInitCursor() {
-    this._updateStops();
-    if (this.initialLineNumber) {
-      this.moveToLineNumber(this.initialLineNumber, this.side);
-      this.initialLineNumber = null;
-    } else {
-      this.moveToFirstChunk();
-    }
-  }
-
-  _handleWindowScroll() {
-    if (this._listeningForScroll) {
-      this._scrollBehavior = ScrollBehavior.NEVER;
-      this._focusOnMove = false;
-      this._listeningForScroll = false;
-    }
-  }
-
-  handleDiffUpdate() {
-    this._updateStops();
     if (!this.diffRow) {
       // does not scroll during init unless requested
       const scrollingBehaviorForInit = this.initialLineNumber ?
         ScrollBehavior.KEEP_VISIBLE :
         ScrollBehavior.NEVER;
       this._scrollBehavior = scrollingBehaviorForInit;
-      this.reInitCursor();
+      if (this.initialLineNumber) {
+        this.moveToLineNumber(this.initialLineNumber, this.side);
+        this.initialLineNumber = null;
+      } else {
+        this.moveToFirstChunk();
+      }
     }
     this._scrollBehavior = ScrollBehavior.KEEP_VISIBLE;
-    this._focusOnMove = true;
-    this._listeningForScroll = false;
+  }
+
+  _handleWindowScroll() {
+    if (this._preventAutoScrollOnManualScroll) {
+      this._scrollBehavior = ScrollBehavior.NEVER;
+      this._focusOnMove = false;
+      this._preventAutoScrollOnManualScroll = false;
+    }
+  }
+
+  handleDiffUpdate() {
+    this._updateStops();
+    this.reInitCursor();
   }
 
   _handleDiffRenderStart() {
-    this._listeningForScroll = true;
+    this._preventAutoScrollOnManualScroll = true;
+  }
+
+  _handleDiffRenderContent() {
+    this._updateStops();
+    // When done rendering, turn focus on move and automatic scrolling back on
+    this._focusOnMove = true;
+    this._preventAutoScrollOnManualScroll = false;
   }
 
   createCommentInPlace() {
@@ -472,7 +476,8 @@ class GrDiffCursor extends mixinBehaviors([Gerrit.FireBehavior],
         i < splice.index + splice.addedCount;
         i++) {
         this.listen(this.diffs[i], 'render-start', '_handleDiffRenderStart');
-        this.listen(this.diffs[i], 'render-content', 'handleDiffUpdate');
+        this.listen(
+            this.diffs[i], 'render-content', '_handleDiffRenderContent');
       }
 
       for (i = 0;
@@ -481,7 +486,7 @@ class GrDiffCursor extends mixinBehaviors([Gerrit.FireBehavior],
         this.unlisten(splice.removed[i],
             'render-start', '_handleDiffRenderStart');
         this.unlisten(splice.removed[i],
-            'render-content', 'handleDiffUpdate');
+            'render-content', '_handleDiffRenderContent');
       }
     }
   }

polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html

@@ -120,17 +120,20 @@ suite('gr-diff-cursor tests', () => {
   });
 
   test('cursor scroll behavior', () => {
-    cursorElement._handleDiffRenderStart();
     assert.equal(cursorElement._scrollBehavior, 'keep-visible');
+
+    cursorElement._handleDiffRenderStart();
     assert.isTrue(cursorElement._focusOnMove);
 
     cursorElement._handleWindowScroll();
     assert.equal(cursorElement._scrollBehavior, 'never');
     assert.isFalse(cursorElement._focusOnMove);
 
-    cursorElement.handleDiffUpdate();
-    assert.equal(cursorElement._scrollBehavior, 'keep-visible');
+    cursorElement._handleDiffRenderContent();
     assert.isTrue(cursorElement._focusOnMove);
+
+    cursorElement.reInitCursor();
+    assert.equal(cursorElement._scrollBehavior, 'keep-visible');
   });
 
   suite('unified diff', () => {
@@ -238,6 +241,7 @@ suite('gr-diff-cursor tests', () => {
 
     function renderHandler() {
       diffElement.removeEventListener('render', renderHandler);
+      cursorElement.reInitCursor();
       assert.isFalse(moveToNumStub.called);
       assert.isTrue(moveToChunkStub.called);
       assert.equal(scrollBehaviorDuringMove, 'never');
@@ -255,6 +259,7 @@ suite('gr-diff-cursor tests', () => {
     const moveToChunkStub = sandbox.stub(cursorElement, 'moveToFirstChunk');
     function renderHandler() {
       diffElement.removeEventListener('render', renderHandler);
+      cursorElement.reInitCursor();
       assert.isFalse(moveToChunkStub.called);
       assert.isTrue(moveToNumStub.called);
       assert.equal(moveToNumStub.lastCall.args[0], 10);
@@ -382,11 +387,11 @@ suite('gr-diff-cursor tests', () => {
   });
 
   test('expand context updates stops', done => {
-    sandbox.spy(cursorElement, 'handleDiffUpdate');
+    sandbox.spy(cursorElement, '_updateStops');
     MockInteractions.tap(diffElement.shadowRoot
         .querySelector('.showContext'));
     flush(() => {
-      assert.isTrue(cursorElement.handleDiffUpdate.called);
+      assert.isTrue(cursorElement._updateStops.called);
       done();
     });
   });

polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js

@@ -314,6 +314,12 @@ class GrDiffView extends mixinBehaviors( [
     this.addEventListener('open-fix-preview',
         this._onOpenFixPreview.bind(this));
     this.$.cursor.push('diffs', this.$.diffHost);
+
+    const onRender = () => {
+      this.$.diffHost.removeEventListener('render', onRender);
+      this.$.cursor.reInitCursor();
+    };
+    this.$.diffHost.addEventListener('render', onRender);
   }
 
   _getLoggedIn() {