Snav: Don't navigate to remote frame

Problem: 1. In AdvanceFocusDirectionallyInContainer(), we process navigable container when the container is either scrollable element or local frame. In addition, we insert an element into already_checked only if the element is local frame. So if the loop processes remote frame as best candidate, then we get stuck in infinite loop. 2. OOPIF.IsKeyboardFocusable() = true && OOPIF.IsNavigableContainer() = true Because OOPIF is focusable, spatial navigation can give focus to OOPIF. If we enter into OOPIF by giving focus it, we can't get out of it by spatial navigation. Solution: Until we support OOPIF correctly, don't navigate to remote frame. To do that, we exclude OOPIF from FocusCandidate. Change-Id: I1928e8c7c9d38951687731a691f0ec0abbcdce10 Reviewed-on: https://chromium-review.googlesource.com/1227853Reviewed-by: Hugo Holgersson <hugoh@vewd.com> Reviewed-by: Fredrik Söderquist <fs@opera.com> Commit-Queue: JunHo Seo <junho0924.seo@lge.com> Cr-Commit-Position: refs/heads/master@{#592739}

Snav: Don't navigate to remote frame
Problem: 1. In AdvanceFocusDirectionallyInContainer(), we process navigable container when the container is either scrollable element or local frame. In addition, we insert an element into already_checked only if the element is local frame. So if the loop processes remote frame as best candidate, then we get stuck in infinite loop. 2. OOPIF.IsKeyboardFocusable() = true && OOPIF.IsNavigableContainer() = true Because OOPIF is focusable, spatial navigation can give focus to OOPIF. If we enter into OOPIF by giving focus it, we can't get out of it by spatial navigation. Solution: Until we support OOPIF correctly, don't navigate to remote frame. To do that, we exclude OOPIF from FocusCandidate. Change-Id: I1928e8c7c9d38951687731a691f0ec0abbcdce10 Reviewed-on: https://chromium-review.googlesource.com/1227853Reviewed-by: Hugo Holgersson <hugoh@vewd.com> Reviewed-by: Fredrik Söderquist <fs@opera.com> Commit-Queue: JunHo Seo <junho0924.seo@lge.com> Cr-Commit-Position: refs/heads/master@{#592739}
41c571bf · JunHo Seo · Commit Bot · 26b98675 · 41c571bf · 41c571bf
Commit 41c571bf authored Sep 20, 2018 by JunHo Seo Committed by Commit Bot Sep 20, 2018
4 changed files
--- a/third_party/blink/renderer/core/page/focus_controller.cc
+++ b/third_party/blink/renderer/core/page/focus_controller.cc
@@ -1368,6 +1368,9 @@ void FocusController::FindFocusCandidateInContainer(
        !IsNavigableContainer(element, direction))
      continue;

+    if (HasRemoteFrame(element))
+      continue;
+
    if (already_checked.Contains(element))
      continue;


--- a/third_party/blink/renderer/core/page/spatial_navigation.cc
+++ b/third_party/blink/renderer/core/page/spatial_navigation.cc
@@ -198,6 +198,15 @@ bool IsRectOffscreen(const Node* node, WebFocusType direction) {
  return !visual_viewport.Intersects(rect);
 }

+bool HasRemoteFrame(const Node* node) {
+  if (!node)
+    return false;
+
+  return node->IsFrameOwnerElement() &&
+         ToHTMLFrameOwnerElement(node)->ContentFrame() &&
+         ToHTMLFrameOwnerElement(node)->ContentFrame()->IsRemoteFrame();
+}
+
 bool ScrollInDirection(LocalFrame* frame, WebFocusType direction) {
  DCHECK(frame);


--- a/third_party/blink/renderer/core/page/spatial_navigation.h
+++ b/third_party/blink/renderer/core/page/spatial_navigation.h
@@ -84,6 +84,7 @@ struct FocusCandidate {
 };

 CORE_EXPORT bool IsRectOffscreen(const Node*, WebFocusType = kWebFocusTypeNone);
+CORE_EXPORT bool HasRemoteFrame(const Node*);
 bool ScrollInDirection(LocalFrame*, WebFocusType);
 bool ScrollInDirection(Node* container, WebFocusType);
 bool IsNavigableContainer(const Node*, WebFocusType);

--- a/third_party/blink/renderer/core/page/spatial_navigation_test.cc
+++ b/third_party/blink/renderer/core/page/spatial_navigation_test.cc
@@ -5,8 +5,12 @@
 #include "third_party/blink/renderer/core/page/spatial_navigation.h"

 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/core/exported/web_remote_frame_impl.h"
+#include "third_party/blink/renderer/core/frame/frame_test_helpers.h"
 #include "third_party/blink/renderer/core/frame/visual_viewport.h"
+#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
 #include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
+#include "third_party/blink/renderer/platform/testing/url_test_helpers.h"

 namespace blink {

@@ -436,4 +440,29 @@ TEST_F(SpatialNavigationTest, TopOfPinchedViewport) {
  EXPECT_GT(origin.Y(), 0);
  EXPECT_EQ(origin, TopOfVisualViewport());
 }
+
+TEST_F(SpatialNavigationTest, HasRemoteFrame) {
+  FrameTestHelpers::WebViewHelper helper;
+  helper.InitializeAndLoad("about:blank", nullptr, nullptr, nullptr, nullptr);
+
+  WebViewImpl* webview = helper.GetWebView();
+  WebURL base_url = URLTestHelpers::ToKURL("http://www.test.com/");
+  FrameTestHelpers::LoadHTMLString(webview->MainFrameImpl(),
+                                   "<!DOCTYPE html>"
+                                   "<iframe id='iframe'></iframe>",
+                                   base_url);
+
+  webview->ResizeWithBrowserControls(IntSize(400, 400), 50, 0, false);
+  webview->MainFrameImpl()->GetFrame()->View()->UpdateAllLifecyclePhases();
+
+  Element* iframe =
+      webview->MainFrameImpl()->GetFrame()->GetDocument()->getElementById(
+          "iframe");
+  EXPECT_FALSE(HasRemoteFrame(iframe));
+
+  webview->MainFrameImpl()->FirstChild()->Swap(
+      FrameTestHelpers::CreateRemote());
+  EXPECT_TRUE(HasRemoteFrame(iframe));
+}
+
 }  // namespace blink