DevTools: fix stick to bottom in console viewport

third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js

Index: third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
diff --git a/third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js b/third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
index f41672c9b35a9187a1f414a30b8484b5c94d47c0..899abf13be91586677ff7237081f51b514ca261a 100644
--- a/third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
+++ b/third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
@@ -154,6 +154,12 @@ WebInspector.ConsoleView = function()
     this._initConsoleMessages();
 
     WebInspector.context.addFlavorChangeListener(WebInspector.ExecutionContext, this._executionContextChanged, this);
+
+    this._messagesElement.addEventListener("mousedown", this._updateStickToBottomOnMouseDown.bind(this), false);
+    this._messagesElement.addEventListener("mousewheel", this._updateStickToBottomOnMouseWheel.bind(this), false);
+    this._messagesElement.addEventListener("mouseup", this._updateStickToBottomOnMouseUp.bind(this), false);
+    this._messagesElement.addEventListener("mouseleave", this._updateStickToBottomOnMouseUp.bind(this), false);
+    this._messagesElement.addEventListener("keydown", this._updateStickToBottomOnKeyDown.bind(this), true);
 }
 
 WebInspector.ConsoleView.persistedHistorySize = 300;
@@ -353,7 +359,7 @@ WebInspector.ConsoleView.prototype = {
 
     restoreScrollPositions: function()
     {
-        if (this._viewport.scrolledToBottom())
+        if (this._viewport.stickToBottom())
             this._immediatelyScrollToBottom();
         else
             WebInspector.Widget.prototype.restoreScrollPositions.call(this);
@@ -363,7 +369,7 @@ WebInspector.ConsoleView.prototype = {
     {
         this._scheduleViewportRefresh();
         this._hidePromptSuggestBox();
-        if (this._viewport.scrolledToBottom())
+        if (this._viewport.stickToBottom())
             this._immediatelyScrollToBottom();
         for (var i = 0; i < this._visibleViewMessages.length; ++i)
             this._visibleViewMessages[i].onResize();
@@ -383,6 +389,8 @@ WebInspector.ConsoleView.prototype = {
          */
         function invalidateViewport()
         {
+            if (this._muteViewportUpdates)
+                return Promise.resolve();
             if (this._needsFullUpdate) {
                 this._updateMessageList();
                 delete this._needsFullUpdate;
@@ -391,12 +399,15 @@ WebInspector.ConsoleView.prototype = {
             }
             return Promise.resolve();
         }
+        if (this._muteViewportUpdates)
+            return;
         this._viewportThrottler.schedule(invalidateViewport.bind(this));
     },
 
     _immediatelyScrollToBottom: function()
     {
         // This will scroll viewport and trigger its refresh.
+        this._viewport.setStickToBottom(true);
         this._promptElement.scrollIntoView(true);
     },
 
@@ -1003,6 +1014,68 @@ WebInspector.ConsoleView.prototype = {
         highlightNode.scrollIntoViewIfNeeded();
     },
 
+    _updateStickToBottomOnMouseDown: function()
+    {
+        this._muteViewportUpdates = true;
+        this._viewport.setStickToBottom(false);
+        this._wasScrolledToBottom = this._messagesElement.isScrolledToBottom();
+    },
+
+    _updateStickToBottomOnMouseUp: function()
+    {
+        this._muteViewportUpdates = false;
+        if (this._wasScrolledToBottom) {
+            // When stuck to the bottom, clicking the gray area above the scroll
+            // thumb should scroll upwards and stop sticking. Smooth scroll
+            // events triggered by the click need to finish before querying
+            // isScrolledToBottom. The timeout value used is the longest smooth
+            // scroll duration, a value retrieved from crbug.com/575409.
+            setTimeout(updateViewportState.bind(this), 200);

[dgozman, 2016/07/30 16:15:54]

I'm worried this timeout is not compatible with fast mouse clicks/drags.

[luoe, 2016/08/01 21:44:37]

A hardcoded value in a timeout worries me too, but I think this is a good
approach.  Bad cases are when the user (1) wants to stick but cannot, or if they
(2) want to un-stick but remain stuck.

Case 1: If the user wants to stick, that means that on mousedown, the bar starts
out above the bottom, so when they are done interacting, the onmouseup code will
check isScrolledToBottom immediately (won't trigger a setTimeout).  It is
possible to end up in a bad case only if a previously scheduled setTimeout calls
updateViewportState when we don't expect.  I can easily reproduce this bad
behavior with:
- start@bottom > Click > keydown immediately (<200ms) after click

Case 2: If the user wants to un-stick, the only way it would stay stuck is if we
think the scroll up has occurred but no scroll events have been processed yet. 
This should only be possible in rare cases if smooth scrolling took longer than
200ms.


Since case 2 *should never happen, I think having bad case 1 is acceptable. 
Clicking and pressing a key within ~200ms is easy to repro, but probably isn't
too common.  If they really intend to stick to the bottom, there will likely be
a sequence of keypresses that take >200ms to type.

In the worst case, users can always guarantee stick/unstick to bottom by waiting
200ms in between their actions.

+        } else {
+            // In another case, when users drag the scroll thumb to the bottom,
+            // the viewport should stick. Checking whether the mouse is released
+            // at the bottom of the viewport is not verifiable after an async
+            // timeout, so we do it immediately.
+            updateViewportState.call(this);
+        }
+        delete this._wasScrolledToBottom;
+
+        /**
+         * @this {!WebInspector.ConsoleView}
+         */
+        function updateViewportState()
+        {
+            this._viewport.setStickToBottom(this._messagesElement.isScrolledToBottom());
+            this._scheduleViewportRefresh();
+            // this._scrollUpdatedByUserForTest();
+        }
+    },
+
+    // _scrollUpdatedByUserForTest: function()
+    // {
+    //     // This method is sniffed in tests.
+    // },
+
+    _updateStickToBottomOnMouseWheel: function()
+    {
+        this._updateStickToBottomOnMouseDown();
+        this._updateStickToBottomOnMouseUp();
+    },
+
+    _updateStickToBottomOnKeyDown: function(event)
+    {
+        var isPageUpDown = event.keyCode === WebInspector.KeyboardShortcut.Keys.PageDown.code || event.keyCode === WebInspector.KeyboardShortcut.Keys.PageUp.code;
+        var isEsc = event.keyCode === WebInspector.KeyboardShortcut.Keys.Esc.code;
+        if (isEsc)

[dgozman, 2016/07/30 16:15:53]

Why this?

[luoe, 2016/08/01 21:44:37]

Any keydown can set stick to bottom == true.  I believe Escape is a special case
because it always toggles the Drawer, which shouldn't signal a jump to the
bottom.

+            return;
+        if (isPageUpDown) {
+            this._updateStickToBottomOnMouseWheel();
+        } else {
+            this._viewport.setStickToBottom(true);
+            // this._scrollUpdatedByUserForTest();
+        }
+    },
+
     __proto__: WebInspector.VBox.prototype
 }