Move arrow scrolling into KeyboardEventManager.

third_party/WebKit/Source/core/input/KeyboardEventManager.cpp

Index: third_party/WebKit/Source/core/input/KeyboardEventManager.cpp
diff --git a/third_party/WebKit/Source/core/input/KeyboardEventManager.cpp b/third_party/WebKit/Source/core/input/KeyboardEventManager.cpp
index eaa25b9ba498ad7754f8eb809e77f5180d1eb79d..b521f0adf1ade7849a27db0411a977d053f65b8a 100644
--- a/third_party/WebKit/Source/core/input/KeyboardEventManager.cpp
+++ b/third_party/WebKit/Source/core/input/KeyboardEventManager.cpp
@@ -8,6 +8,7 @@
 #include "core/editing/Editor.h"
 #include "core/events/KeyboardEvent.h"
 #include "core/html/HTMLDialogElement.h"
+#include "core/input/EventHandler.h"
 #include "core/input/EventHandlingUtil.h"
 #include "core/input/ScrollManager.h"
 #include "core/layout/LayoutObject.h"
@@ -17,6 +18,7 @@
 #include "core/page/FocusController.h"
 #include "core/page/Page.h"
 #include "core/page/SpatialNavigation.h"
+#include "platform/KeyboardCodes.h"
 #include "platform/UserGestureIndicator.h"
 #include "platform/WindowsKeyboardCodes.h"
 #include "public/platform/WebInputEvent.h"
@@ -50,6 +52,71 @@ WebFocusType focusDirectionForKey(const String& key) {
   return retVal;
 }
 
+bool mapKeyCodeForScroll(int keyCode,
+                         PlatformEvent::Modifiers modifiers,
+                         ScrollDirection* scrollDirection,
+                         ScrollGranularity* scrollGranularity) {
+  if (modifiers & PlatformEvent::ShiftKey || modifiers & PlatformEvent::MetaKey)
+    return false;
+
+  if (modifiers & PlatformEvent::AltKey) {
+    // Alt-Up/Down should behave like PageUp/Down on Mac.  (Note that Alt-keys
+    // on other platforms are suppressed due to isSystemKey being set.)

[skobes, 2016/10/26 17:57:44]

Why do Alt-keys not set isSystemKey on Mac?

I wonder if it's safer to leave the #if guard on this, since it is Mac specific.

[aelias_OOO_until_Jul13, 2016/10/26 19:27:39]

On Mac, the system key is the meta key instead:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/web_...

I have a strong bias against #if guards because they make unit testing much more
painful (issues only appear on platform trybots, not locally).  AFAIK this is
safe and well-aligned with the intent of the platform-specific suppression.  If
there's a regression reported out of this, I'll be happy to fix.

+    if (keyCode == VKEY_UP)
+      keyCode = VKEY_PRIOR;
+    else if (keyCode == VKEY_DOWN)
+      keyCode = VKEY_NEXT;
+    else
+      return false;
+  }
+
+  if (modifiers & PlatformEvent::CtrlKey) {
+    // Match FF behavior in the sense that Ctrl+home/end are the only Ctrl
+    // key combinations which affect scrolling.
+    if (keyCode != VKEY_HOME && keyCode != VKEY_END)
+      return false;
+  }
+
+  switch (keyCode) {
+    case VKEY_LEFT:
+      *scrollDirection = ScrollLeftIgnoringWritingMode;
+      *scrollGranularity = ScrollByLine;
+      break;
+    case VKEY_RIGHT:
+      *scrollDirection = ScrollRightIgnoringWritingMode;
+      *scrollGranularity = ScrollByLine;
+      break;
+    case VKEY_UP:
+      *scrollDirection = ScrollUpIgnoringWritingMode;
+      *scrollGranularity = ScrollByLine;
+      break;
+    case VKEY_DOWN:
+      *scrollDirection = ScrollDownIgnoringWritingMode;
+      *scrollGranularity = ScrollByLine;
+      break;
+    case VKEY_HOME:
+      *scrollDirection = ScrollUpIgnoringWritingMode;
+      *scrollGranularity = ScrollByDocument;
+      break;
+    case VKEY_END:
+      *scrollDirection = ScrollDownIgnoringWritingMode;
+      *scrollGranularity = ScrollByDocument;
+      break;
+    case VKEY_PRIOR:  // page up
+      *scrollDirection = ScrollUpIgnoringWritingMode;
+      *scrollGranularity = ScrollByPage;
+      break;
+    case VKEY_NEXT:  // page down
+      *scrollDirection = ScrollDownIgnoringWritingMode;
+      *scrollGranularity = ScrollByPage;
+      break;
+    default:
+      return false;
+  }
+
+  return true;
+}
+
 }  // namespace
 
 KeyboardEventManager::KeyboardEventManager(LocalFrame* frame,
@@ -210,9 +277,7 @@ void KeyboardEventManager::defaultKeyboardEventHandler(
     } else if (event->key() == "Escape") {
       defaultEscapeEventHandler(event);
     } else {
-      WebFocusType type = focusDirectionForKey(event->key());
-      if (type != WebFocusTypeNone)
-        defaultArrowEventHandler(type, event);
+      defaultArrowEventHandler(event, possibleFocusedNode);
     }
   }
   if (event->type() == EventTypeNames::keypress) {
@@ -263,27 +328,39 @@ void KeyboardEventManager::defaultBackspaceEventHandler(KeyboardEvent* event) {
     event->setDefaultHandled();
 }
 
-void KeyboardEventManager::defaultArrowEventHandler(WebFocusType focusType,
-                                                    KeyboardEvent* event) {
+void KeyboardEventManager::defaultArrowEventHandler(KeyboardEvent* event,
+                                                    Node* possibleFocusedNode) {
   DCHECK_EQ(event->type(), EventTypeNames::keydown);
 
-  if (event->ctrlKey() || event->metaKey() || event->shiftKey())
-    return;
-
   Page* page = m_frame->page();
   if (!page)
     return;
 
-  if (!isSpatialNavigationEnabled(m_frame))
+  WebFocusType type = focusDirectionForKey(event->key());
+  if (event->ctrlKey() || event->metaKey() || event->shiftKey())

[skobes, 2016/10/26 17:57:44]

Maybe this check should be in focusDirectionForKey (and pass it the event).

[aelias_OOO_until_Jul13, 2016/10/26 19:27:39]

Done.

+    type = WebFocusTypeNone;
+  if (type != WebFocusTypeNone && isSpatialNavigationEnabled(m_frame) &&
+      !m_frame->document()->inDesignMode()) {
+    if (page->focusController().advanceFocus(type)) {
+      event->setDefaultHandled();
+      return;
+    }
+  }
+
+  if (event->keyEvent()->isSystemKey)
     return;
 
-  // Arrows and other possible directional navigation keys can be used in design
-  // mode editing.
-  if (m_frame->document()->inDesignMode())
+  ScrollDirection scrollDirection;
+  ScrollGranularity scrollGranularity;
+  if (!mapKeyCodeForScroll(event->keyCode(), event->modifiers(),
+                           &scrollDirection, &scrollGranularity))
     return;
 
-  if (page->focusController().advanceFocus(focusType))
+  if (m_scrollManager->bubblingScroll(scrollDirection, scrollGranularity,
+                                      nullptr, possibleFocusedNode)) {
     event->setDefaultHandled();
+    return;
+  }
 }
 
 void KeyboardEventManager::defaultTabEventHandler(KeyboardEvent* event) {