Move last NavigationPolicy decisions to navigation_policy.cc

Bug: 849055
Change-Id: Ie60a938bc8831d149cd13c796e0089a5c3ccd988
Reviewed-on: https://chromium-review.googlesource.com/1100309
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567657}

third_party/blink/renderer/core/BUILD.gn

@@ -1995,6 +1995,7 @@ jumbo_source_set("unit_tests") {
     "loader/mixed_content_checker_test.cc",
     "loader/modulescript/module_script_loader_test.cc",
     "loader/modulescript/module_tree_linker_test.cc",
+    "loader/navigation_policy_test.cc",
     "loader/ping_loader_test.cc",
     "loader/programmatic_scroll_test.cc",
     "loader/progress_tracker_test.cc",
@@ -2015,7 +2016,6 @@ jumbo_source_set("unit_tests") {
     "page/chrome_client_test.cc",
     "page/context_menu_controller_test.cc",
     "page/drag_controller_test.cc",
-    "page/effective_navigation_policy_test.cc",
     "page/focus_controller_test.cc",
     "page/page_overlay_test.cc",
     "page/page_popup_client_test.cc",

third_party/blink/renderer/core/events/current_input_event.h

@@ -5,13 +5,13 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_EVENTS_CURRENT_INPUT_EVENT_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_EVENTS_CURRENT_INPUT_EVENT_H_
 
+#include "third_party/blink/renderer/core/core_export.h"
+
 namespace blink {
 
-class WebFrameWidgetImpl;
 class WebInputEvent;
-class WebViewImpl;
 
-class CurrentInputEvent {
+class CORE_EXPORT CurrentInputEvent {
  public:
   // Gets the "current" input event - event that is currently being processed by
   // either blink::WebViewImpl::HandleInputEventInternal or by
@@ -21,6 +21,8 @@ class CurrentInputEvent {
  private:
   friend class WebViewImpl;
   friend class WebFrameWidgetImpl;
+  friend class NavigationPolicyTest;
+
   static const WebInputEvent* current_input_event_;
 };
 

third_party/blink/renderer/core/loader/navigation_policy.cc

@@ -40,7 +40,6 @@
 #include "third_party/blink/renderer/core/events/keyboard_event.h"
 #include "third_party/blink/renderer/core/events/mouse_event.h"
 #include "third_party/blink/renderer/core/events/ui_event_with_key_state.h"
-#include "third_party/blink/renderer/core/page/create_window.h"
 #include "third_party/blink/renderer/platform/keyboard_codes.h"
 #include "third_party/blink/renderer/platform/wtf/assertions.h"
 
@@ -100,31 +99,8 @@ NavigationPolicy NavigationPolicyFromEventInternal(Event* event) {
   return kNavigationPolicyCurrentTab;
 }
 
-}  // namespace
-
-NavigationPolicy NavigationPolicyFromEvent(Event* event) {
-  NavigationPolicy event_policy = NavigationPolicyFromEventInternal(event);
-  NavigationPolicy input_policy =
-      NavigationPolicyFromEvent(CurrentInputEvent::Get());
-
-  if (event_policy == kNavigationPolicyDownload &&
-      input_policy != kNavigationPolicyDownload) {
-    // No downloads from synthesized events without user intention.
-    return kNavigationPolicyCurrentTab;
-  }
-
-  if (event_policy == kNavigationPolicyNewBackgroundTab &&
-      input_policy != kNavigationPolicyNewBackgroundTab &&
-      !UIEventWithKeyState::NewTabModifierSetFromIsolatedWorld()) {
-    // No "tab-unders" from synthesized events without user intention.
-    // Events originating from an isolated world are exempt.
-    return kNavigationPolicyNewForegroundTab;
-  }
-
-  return event_policy;
-}
-
-NavigationPolicy NavigationPolicyFromEvent(const WebInputEvent* event) {
+NavigationPolicy NavigationPolicyFromCurrentEvent() {
+  const WebInputEvent* event = CurrentInputEvent::Get();
   if (!event)
     return kNavigationPolicyCurrentTab;
 
@@ -162,6 +138,62 @@ NavigationPolicy NavigationPolicyFromEvent(const WebInputEvent* event) {
       event->GetModifiers() & WebInputEvent::kMetaKey);
 }
 
+}  // namespace
+
+NavigationPolicy NavigationPolicyFromEvent(Event* event) {
+  NavigationPolicy event_policy = NavigationPolicyFromEventInternal(event);
+  NavigationPolicy input_policy = NavigationPolicyFromCurrentEvent();
+
+  if (event_policy == kNavigationPolicyDownload &&
+      input_policy != kNavigationPolicyDownload) {
+    // No downloads from synthesized events without user intention.
+    return kNavigationPolicyCurrentTab;
+  }
+
+  if (event_policy == kNavigationPolicyNewBackgroundTab &&
+      input_policy != kNavigationPolicyNewBackgroundTab &&
+      !UIEventWithKeyState::NewTabModifierSetFromIsolatedWorld()) {
+    // No "tab-unders" from synthesized events without user intention.
+    // Events originating from an isolated world are exempt.
+    return kNavigationPolicyNewForegroundTab;
+  }
+
+  return event_policy;
+}
+
+NavigationPolicy NavigationPolicyForCreateWindow(
+    const WebWindowFeatures& features) {
+  // If our default configuration was modified by a script or wasn't
+  // created by a user gesture, then show as a popup. Else, let this
+  // new window be opened as a toplevel window.
+  bool as_popup = !features.tool_bar_visible || !features.status_bar_visible ||
+                  !features.scrollbars_visible || !features.menu_bar_visible ||
+                  !features.resizable;
+  NavigationPolicy app_policy =
+      as_popup ? kNavigationPolicyNewPopup : kNavigationPolicyNewForegroundTab;
+  NavigationPolicy user_policy = NavigationPolicyFromCurrentEvent();
+
+  if (user_policy == kNavigationPolicyNewWindow &&
+      app_policy == kNavigationPolicyNewPopup) {
+    // User and app agree that we want a new window; let the app override the
+    // decorations.
+    return app_policy;
+  }
+
+  if (user_policy == kNavigationPolicyCurrentTab) {
+    // User doesn't want a specific policy, use app policy instead.
+    return app_policy;
+  }
+
+  if (user_policy == kNavigationPolicyDownload) {
+    // When the input event suggests a download, but the navigation was
+    // initiated by script, we should not override it.
+    return app_policy;
+  }
+
+  return user_policy;
+}
+
 STATIC_ASSERT_ENUM(kWebNavigationPolicyIgnore, kNavigationPolicyIgnore);
 STATIC_ASSERT_ENUM(kWebNavigationPolicyDownload, kNavigationPolicyDownload);
 STATIC_ASSERT_ENUM(kWebNavigationPolicyCurrentTab, kNavigationPolicyCurrentTab);

third_party/blink/renderer/core/loader/navigation_policy.h

@@ -36,7 +36,7 @@
 namespace blink {
 
 class Event;
-class WebInputEvent;
+struct WebWindowFeatures;
 
 enum NavigationPolicy {
   kNavigationPolicyIgnore,
@@ -56,10 +56,11 @@ enum NavigationPolicy {
 // or new tabs without user intention coming from a real input event.
 CORE_EXPORT NavigationPolicy NavigationPolicyFromEvent(Event*);
 
-// This is a helper method which returns policy for a real input event
-// from the user.
-// TODO(dgozman): this function should be gone soon.
-CORE_EXPORT NavigationPolicy NavigationPolicyFromEvent(const WebInputEvent*);
+// Returns a NavigationPolicy to use for navigating a new window.
+// This function respects user intention coming from a real input event,
+// and ensures that we don't perform a download instead of navigation.
+CORE_EXPORT NavigationPolicy
+NavigationPolicyForCreateWindow(const WebWindowFeatures&);
 
 }  // namespace blink
 

third_party/blink/renderer/core/page/effective_navigation_policy_test.cc → third_party/blink/renderer/core/loader/navigation_policy_test.cc

@@ -28,65 +28,69 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include "third_party/blink/renderer/core/loader/navigation_policy.h"
+#include "base/auto_reset.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/platform/web_input_event.h"
 #include "third_party/blink/public/platform/web_mouse_event.h"
 #include "third_party/blink/public/web/web_window_features.h"
-#include "third_party/blink/renderer/core/page/create_window.h"
+#include "third_party/blink/renderer/core/events/current_input_event.h"
 
 namespace blink {
 
-class EffectiveNavigationPolicyTest : public testing::Test {
+// TODO(dgozman): add tests for NavigationPolicyFromEvent.
+class NavigationPolicyTest : public testing::Test {
  protected:
-  NavigationPolicy GetNavigationPolicyWithMouseEvent(
-      int modifiers,
-      WebMouseEvent::Button button,
-      bool as_popup) {
+  NavigationPolicy GetPolicyForCreateWindow(int modifiers,
+                                            WebMouseEvent::Button button,
+                                            bool as_popup) {
     WebMouseEvent event(WebInputEvent::kMouseUp, modifiers,
                         WebInputEvent::GetStaticTimeStampForTests());
     event.button = button;
     if (as_popup)
       features.tool_bar_visible = false;
-    return EffectiveNavigationPolicy(&event, features);
+    base::AutoReset<const WebInputEvent*> current_event_change(
+        &CurrentInputEvent::current_input_event_, &event);
+    return NavigationPolicyForCreateWindow(features);
   }
 
   WebWindowFeatures features;
 };
 
-TEST_F(EffectiveNavigationPolicyTest, LeftClick) {
+TEST_F(NavigationPolicyTest, LeftClick) {
   int modifiers = 0;
   WebMouseEvent::Button button = WebMouseEvent::Button::kLeft;
   bool as_popup = false;
   EXPECT_EQ(kNavigationPolicyNewForegroundTab,
-            GetNavigationPolicyWithMouseEvent(modifiers, button, as_popup));
+            GetPolicyForCreateWindow(modifiers, button, as_popup));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, LeftClickPopup) {
+TEST_F(NavigationPolicyTest, LeftClickPopup) {
   int modifiers = 0;
   WebMouseEvent::Button button = WebMouseEvent::Button::kLeft;
   bool as_popup = true;
   EXPECT_EQ(kNavigationPolicyNewPopup,
-            GetNavigationPolicyWithMouseEvent(modifiers, button, as_popup));
+            GetPolicyForCreateWindow(modifiers, button, as_popup));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, ShiftLeftClick) {
+TEST_F(NavigationPolicyTest, ShiftLeftClick) {
   int modifiers = WebInputEvent::kShiftKey;
   WebMouseEvent::Button button = WebMouseEvent::Button::kLeft;
   bool as_popup = false;
   EXPECT_EQ(kNavigationPolicyNewWindow,
-            GetNavigationPolicyWithMouseEvent(modifiers, button, as_popup));
+            GetPolicyForCreateWindow(modifiers, button, as_popup));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, ShiftLeftClickPopup) {
+TEST_F(NavigationPolicyTest, ShiftLeftClickPopup) {
   int modifiers = WebInputEvent::kShiftKey;
   WebMouseEvent::Button button = WebMouseEvent::Button::kLeft;
   bool as_popup = true;
   EXPECT_EQ(kNavigationPolicyNewPopup,
-            GetNavigationPolicyWithMouseEvent(modifiers, button, as_popup));
+            GetPolicyForCreateWindow(modifiers, button, as_popup));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, ControlOrMetaLeftClick) {
+TEST_F(NavigationPolicyTest, ControlOrMetaLeftClick) {
 #if defined(OS_MACOSX)
   int modifiers = WebInputEvent::kMetaKey;
 #else
@@ -95,10 +99,10 @@ TEST_F(EffectiveNavigationPolicyTest, ControlOrMetaLeftClick) {
   WebMouseEvent::Button button = WebMouseEvent::Button::kLeft;
   bool as_popup = false;
   EXPECT_EQ(kNavigationPolicyNewBackgroundTab,
-            GetNavigationPolicyWithMouseEvent(modifiers, button, as_popup));
+            GetPolicyForCreateWindow(modifiers, button, as_popup));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, ControlOrMetaLeftClickPopup) {
+TEST_F(NavigationPolicyTest, ControlOrMetaLeftClickPopup) {
 #if defined(OS_MACOSX)
   int modifiers = WebInputEvent::kMetaKey;
 #else
@@ -107,10 +111,10 @@ TEST_F(EffectiveNavigationPolicyTest, ControlOrMetaLeftClickPopup) {
   WebMouseEvent::Button button = WebMouseEvent::Button::kLeft;
   bool as_popup = true;
   EXPECT_EQ(kNavigationPolicyNewBackgroundTab,
-            GetNavigationPolicyWithMouseEvent(modifiers, button, as_popup));
+            GetPolicyForCreateWindow(modifiers, button, as_popup));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, ControlOrMetaAndShiftLeftClick) {
+TEST_F(NavigationPolicyTest, ControlOrMetaAndShiftLeftClick) {
 #if defined(OS_MACOSX)
   int modifiers = WebInputEvent::kMetaKey;
 #else
@@ -120,10 +124,10 @@ TEST_F(EffectiveNavigationPolicyTest, ControlOrMetaAndShiftLeftClick) {
   WebMouseEvent::Button button = WebMouseEvent::Button::kLeft;
   bool as_popup = false;
   EXPECT_EQ(kNavigationPolicyNewForegroundTab,
-            GetNavigationPolicyWithMouseEvent(modifiers, button, as_popup));
+            GetPolicyForCreateWindow(modifiers, button, as_popup));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, ControlOrMetaAndShiftLeftClickPopup) {
+TEST_F(NavigationPolicyTest, ControlOrMetaAndShiftLeftClickPopup) {
 #if defined(OS_MACOSX)
   int modifiers = WebInputEvent::kMetaKey;
 #else
@@ -133,59 +137,59 @@ TEST_F(EffectiveNavigationPolicyTest, ControlOrMetaAndShiftLeftClickPopup) {
   WebMouseEvent::Button button = WebMouseEvent::Button::kLeft;
   bool as_popup = true;
   EXPECT_EQ(kNavigationPolicyNewForegroundTab,
-            GetNavigationPolicyWithMouseEvent(modifiers, button, as_popup));
+            GetPolicyForCreateWindow(modifiers, button, as_popup));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, MiddleClick) {
+TEST_F(NavigationPolicyTest, MiddleClick) {
   int modifiers = 0;
   bool as_popup = false;
   WebMouseEvent::Button button = WebMouseEvent::Button::kMiddle;
   EXPECT_EQ(kNavigationPolicyNewBackgroundTab,
-            GetNavigationPolicyWithMouseEvent(modifiers, button, as_popup));
+            GetPolicyForCreateWindow(modifiers, button, as_popup));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, MiddleClickPopup) {
+TEST_F(NavigationPolicyTest, MiddleClickPopup) {
   int modifiers = 0;
   bool as_popup = true;
   WebMouseEvent::Button button = WebMouseEvent::Button::kMiddle;
   EXPECT_EQ(kNavigationPolicyNewBackgroundTab,
-            GetNavigationPolicyWithMouseEvent(modifiers, button, as_popup));
+            GetPolicyForCreateWindow(modifiers, button, as_popup));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, NoToolbarsForcesPopup) {
+TEST_F(NavigationPolicyTest, NoToolbarsForcesPopup) {
   features.tool_bar_visible = false;
   EXPECT_EQ(kNavigationPolicyNewPopup,
-            EffectiveNavigationPolicy(nullptr, features));
+            NavigationPolicyForCreateWindow(features));
   features.tool_bar_visible = true;
   EXPECT_EQ(kNavigationPolicyNewForegroundTab,
-            EffectiveNavigationPolicy(nullptr, features));
+            NavigationPolicyForCreateWindow(features));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, NoStatusBarForcesPopup) {
+TEST_F(NavigationPolicyTest, NoStatusBarForcesPopup) {
   features.status_bar_visible = false;
   EXPECT_EQ(kNavigationPolicyNewPopup,
-            EffectiveNavigationPolicy(nullptr, features));
+            NavigationPolicyForCreateWindow(features));
   features.status_bar_visible = true;
   EXPECT_EQ(kNavigationPolicyNewForegroundTab,
-            EffectiveNavigationPolicy(nullptr, features));
+            NavigationPolicyForCreateWindow(features));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, NoMenuBarForcesPopup) {
+TEST_F(NavigationPolicyTest, NoMenuBarForcesPopup) {
   features.menu_bar_visible = false;
   EXPECT_EQ(kNavigationPolicyNewPopup,
-            EffectiveNavigationPolicy(nullptr, features));
+            NavigationPolicyForCreateWindow(features));
   features.menu_bar_visible = true;
   EXPECT_EQ(kNavigationPolicyNewForegroundTab,
-            EffectiveNavigationPolicy(nullptr, features));
+            NavigationPolicyForCreateWindow(features));
 }
 
-TEST_F(EffectiveNavigationPolicyTest, NotResizableForcesPopup) {
+TEST_F(NavigationPolicyTest, NotResizableForcesPopup) {
   features.resizable = false;
   EXPECT_EQ(kNavigationPolicyNewPopup,
-            EffectiveNavigationPolicy(nullptr, features));
+            NavigationPolicyForCreateWindow(features));
   features.resizable = true;
   EXPECT_EQ(kNavigationPolicyNewForegroundTab,
-            EffectiveNavigationPolicy(nullptr, features));
+            NavigationPolicyForCreateWindow(features));
 }
 
 }  // namespace blink

third_party/blink/renderer/core/page/create_window.cc

@@ -31,8 +31,6 @@
 #include "third_party/blink/public/platform/web_url_request.h"
 #include "third_party/blink/public/web/web_window_features.h"
 #include "third_party/blink/renderer/core/dom/document.h"
-#include "third_party/blink/renderer/core/events/current_input_event.h"
-#include "third_party/blink/renderer/core/events/ui_event_with_key_state.h"
 #include "third_party/blink/renderer/core/exported/web_view_impl.h"
 #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
 #include "third_party/blink/renderer/core/frame/frame_client.h"
@@ -51,42 +49,6 @@
 
 namespace blink {
 
-// Check that the desired NavigationPolicy |policy| is compatible with the
-// observed input event |current_event|.
-// TODO(dgozman): move this to navigation_policy.cc.
-NavigationPolicy EffectiveNavigationPolicy(const WebInputEvent* current_event,
-                                           const WebWindowFeatures& features) {
-  // If our default configuration was modified by a script or wasn't
-  // created by a user gesture, then show as a popup. Else, let this
-  // new window be opened as a toplevel window.
-  bool as_popup = !features.tool_bar_visible || !features.status_bar_visible ||
-                  !features.scrollbars_visible || !features.menu_bar_visible ||
-                  !features.resizable;
-  NavigationPolicy app_policy =
-      as_popup ? kNavigationPolicyNewPopup : kNavigationPolicyNewForegroundTab;
-  NavigationPolicy user_policy = NavigationPolicyFromEvent(current_event);
-
-  if (user_policy == kNavigationPolicyNewWindow &&
-      app_policy == kNavigationPolicyNewPopup) {
-    // User and app agree that we want a new window; let the app override the
-    // decorations.
-    return app_policy;
-  }
-
-  if (user_policy == kNavigationPolicyCurrentTab) {
-    // User doesn't want a specific policy, use app policy instead.
-    return app_policy;
-  }
-
-  if (user_policy == kNavigationPolicyDownload) {
-    // When the input event suggests a download, but the navigation was
-    // initiated by script, we should not override it.
-    return app_policy;
-  }
-
-  return user_policy;
-}
-
 // Though isspace() considers \t and \v to be whitespace, Win IE doesn't when
 // parsing window features.
 static bool IsWindowFeaturesSeparator(UChar c) {
@@ -242,10 +204,9 @@ static Frame* CreateNewWindow(LocalFrame& opener_frame,
   if (!old_page)
     return nullptr;
 
-  NavigationPolicy policy =
-      force_new_foreground_tab
-          ? kNavigationPolicyNewForegroundTab
-          : EffectiveNavigationPolicy(CurrentInputEvent::Get(), features);
+  NavigationPolicy policy = force_new_foreground_tab
+                                ? kNavigationPolicyNewForegroundTab
+                                : NavigationPolicyForCreateWindow(features);
 
   const SandboxFlags sandbox_flags =
       opener_frame.GetDocument()->IsSandboxed(

third_party/blink/renderer/core/page/create_window.h

@@ -51,11 +51,6 @@ DOMWindow* CreateWindow(const String& url_string,
 
 void CreateWindowForRequest(const FrameLoadRequest&, LocalFrame& opener_frame);
 
-// Exposed for testing
-CORE_EXPORT NavigationPolicy
-EffectiveNavigationPolicy(const WebInputEvent* current_event,
-                          const WebWindowFeatures&);
-
 // Exposed for testing
 CORE_EXPORT WebWindowFeatures GetWindowFeaturesFromString(const String&);