OOPIF: Enable TabAndMouseFocusNavigation.

third_party/WebKit/Source/core/page/FocusController.cpp

Index: third_party/WebKit/Source/core/page/FocusController.cpp
diff --git a/third_party/WebKit/Source/core/page/FocusController.cpp b/third_party/WebKit/Source/core/page/FocusController.cpp
index 383f1623b0b046285b32c9e0537673b3ddb79e4d..2aba7a05069f65455ba69d08b4fa4b31b013de14 100644
--- a/third_party/WebKit/Source/core/page/FocusController.cpp
+++ b/third_party/WebKit/Source/core/page/FocusController.cpp
@@ -933,7 +933,11 @@ bool FocusController::advanceFocusAcrossFrames(
     start = toHTMLFrameOwnerElement(from->owner());
   }
 
-  return advanceFocusInDocumentOrder(to, start, type, false,
+  // If we're coming from a parent frame, we need to restart from the first or
+  // last focusable element.
+  bool initialFocus = to->tree().parent() == from;
+
+  return advanceFocusInDocumentOrder(to, start, type, initialFocus,
                                      sourceCapabilities);
 }
 
@@ -999,6 +1003,8 @@ bool FocusController::advanceFocusInDocumentOrder(
 
   if (element == document->focusedElement()) {
     // Focus wrapped around to the same element.
+    setFocusedFrame(document->frame());
+    dispatchFocusEvent(*document, *element);

[alexmos, 2017/04/05 00:21:13]

Sanity check: could this have any side effects (like an extra focus event) in
non-OOPIF modes?  Doesn't seem likely, as ChromeClientImpl::canTakeFocus always
returns true, unless we're in layout test mode, so realistically I don't see how
it's possible to press tab from a focused element and wrap around to that same
focused element (since we'd return early after the canTakeFocus check above.)

[avallee, 2017/04/05 15:13:00]

It could mean an extra focus event, but I don't know how this would be
constructed. If there's a single element and we tab through, we'd ask the
embedder to TakeFocus(). This wrapping around doesn't seem like a problem. 
Maybe there's a way to listen for a tab event that would focus the next element
and then have the default handlers advance focus onto the same element?

This early out is exactly how we are getting trapped with the change in focused
frame in the case where we steal focus and the document doesn't clear the
focused element,

Also, there seems to be no test to verify that this case would not result in a
focus event. The cl passes dry run so I'm feeling decently confident that we're
fine or we'll break everything and end up with a test case.

[alexmos, 2017/04/05 18:08:33]

Acknowledged.

     return true;
   }
 
@@ -1012,7 +1018,6 @@ bool FocusController::advanceFocusInDocumentOrder(
       return false;
 
     document->clearFocusedElement();
-    setFocusedFrame(owner->contentFrame());
 
     // If contentFrame is remote, continue the search for focusable
     // elements in that frame's process.

[alexmos, 2017/04/05 00:21:13]

Might be worth also mentioning that contentFrame will be set as the focused
frame from its own process.  

(is this right?  I think the call that would do this is here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/page/Focu...)

[avallee, 2017/04/05 15:13:00]

Yes, it would either be set up above where your other comment is, or down below
near the final return.

Done.

@@ -1020,6 +1025,8 @@ bool FocusController::advanceFocusInDocumentOrder(
     // contentFrame, hence the need to null-check it again.
     if (owner->contentFrame() && owner->contentFrame()->isRemoteFrame())
       toRemoteFrame(owner->contentFrame())->advanceFocus(type, frame);
+    else
+      setFocusedFrame(owner->contentFrame());
 
     return true;
   }