Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(568)

Unified Diff: third_party/WebKit/Source/core/page/FocusController.cpp

Issue 1500873002: Implement sequential focus navigation for OOPIF. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Cleanup and test fix Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 4bde2a973602ff3a8b47548ee964adbf8b974499..d0a97c766d642b7b795dde89cc6186e43e788ff7 100644
--- a/third_party/WebKit/Source/core/page/FocusController.cpp
+++ b/third_party/WebKit/Source/core/page/FocusController.cpp
@@ -43,6 +43,7 @@
#include "core/frame/FrameView.h"
#include "core/frame/LocalDOMWindow.h"
#include "core/frame/LocalFrame.h"
+#include "core/frame/RemoteFrame.h"
#include "core/frame/Settings.h"
#include "core/html/HTMLAreaElement.h"
#include "core/html/HTMLImageElement.h"
@@ -690,8 +691,13 @@ bool FocusController::advanceFocus(WebFocusType type, bool initialFocus, InputDe
{
switch (type) {
case WebFocusTypeForward:
- case WebFocusTypeBackward:
- return advanceFocusInDocumentOrder(type, initialFocus, sourceCapabilities);
+ case WebFocusTypeBackward: {
+ // We should never hit this when a RemoteFrame is focused, since the key
+ // event that initiated focus advancement should've been routed to that
+ // frame's process from the beginning.
+ LocalFrame* startingFrame = toLocalFrame(focusedOrMainFrame());
+ return advanceFocusInDocumentOrder(startingFrame, nullptr, type, initialFocus, sourceCapabilities);
+ }
case WebFocusTypeLeft:
case WebFocusTypeRight:
case WebFocusTypeUp:
@@ -704,17 +710,30 @@ bool FocusController::advanceFocus(WebFocusType type, bool initialFocus, InputDe
return false;
}
-bool FocusController::advanceFocusInDocumentOrder(WebFocusType type, bool initialFocus, InputDeviceCapabilities* sourceCapabilities)
+bool FocusController::advanceFocusAcrossFrames(WebFocusType type, RemoteFrame* from, LocalFrame* to, InputDeviceCapabilities* sourceCapabilities)
+{
+ // If we are shifting focus from a child frame to its parent, the
+ // child frame has no more focusable elements, and we should continue
+ // looking for focusable elements in the parent, starting from the <iframe>
+ // element of the child frame.
+ Node* startingNode = nullptr;
+ if (from->tree().parent() == to) {
+ ASSERT(from->owner()->isLocal());
+ startingNode = toHTMLFrameOwnerElement(from->owner());
+ }
+
+ return advanceFocusInDocumentOrder(to, startingNode, type, false, sourceCapabilities);
+}
+
+bool FocusController::advanceFocusInDocumentOrder(LocalFrame* frame, Node* startingNode, WebFocusType type, bool initialFocus, InputDeviceCapabilities* sourceCapabilities)
{
- // FIXME: Focus advancement won't work with externally rendered frames until after
- // inter-frame focus control is moved out of Blink.
- if (!focusedOrMainFrame()->isLocalFrame())
- return false;
- LocalFrame* frame = toLocalFrame(focusedOrMainFrame());
ASSERT(frame);
Document* document = frame->document();
- Node* currentNode = document->focusedElement();
+ Node* currentNode = startingNode;
+ if (!currentNode)
+ currentNode = document->focusedElement();
+
// FIXME: Not quite correct when it comes to focus transitions leaving/entering the WebView itself
bool caretBrowsing = frame->settings() && frame->settings()->caretBrowsingEnabled();
@@ -726,6 +745,14 @@ bool FocusController::advanceFocusInDocumentOrder(WebFocusType type, bool initia
RefPtrWillBeRawPtr<Element> element = findFocusableElementAcrossFocusScopes(type, FocusNavigationScope::focusNavigationScopeOf(currentNode ? *currentNode : *document), currentNode);
if (!element) {
+ // If there's a RemoteFrame on the ancestor chain, we need to continue
+ // searching for focusable elements there.
+ if (frame->localFrameRoot() != frame->tree().top()) {
+ document->clearFocusedElement();
+ toRemoteFrame(frame->localFrameRoot()->tree().parent())->advanceFocus(type, frame->localFrameRoot());
+ return true;
+ }
+
// We didn't find an element to focus, so we should try to pass focus to Chrome.
if (!initialFocus && m_page->chromeClient().canTakeFocus(type)) {
document->clearFocusedElement();
@@ -760,6 +787,14 @@ bool FocusController::advanceFocusInDocumentOrder(WebFocusType type, bool initia
document->clearFocusedElement();
setFocusedFrame(owner->contentFrame());
+
+ // If contentFrame is remote, continue the search for focusable
+ // elements in that frame's process.
+ // clearFocusedElement() fires events that might detach the
+ // contentFrame, hence the need to null-check it again.
+ if (owner->contentFrame() && owner->contentFrame()->isRemoteFrame())
+ toRemoteFrame(owner->contentFrame())->advanceFocus(type, frame);
+
return true;
}

Powered by Google App Engine
This is Rietveld 408576698