Initiate drag-and-drop navigation through a local frame.

Since r774391 the dropped link opens in a new tab (via
NavigationPolicy::kNavigationPolicyNewForegroundTab).  This means that
it doesn't matter which frame is used to initiate the navigation, since
the navigation should always translate into an OpenURL with
WindowOpenDisposition::NEW_FOREGROUND_TAB and handled in the new
window/frame.

After this CL, the navigation will be always initiated through the local
frame that is handling the dropped URL.  This helps avoid
RemoteFrame::Navigate which doesn't know how to handle NavigationPolicy.
and let's us restore the test asserts in the
DragAndDropBrowserTest.DropValidUrlFromOutside test.

Fixed: 1087898
Change-Id: Icc6b2d8da37f181a32663917a02850a72d9c5297
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290705
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Dana Fried <dfried@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788370}

chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc

@@ -10,6 +10,7 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/sequenced_task_runner.h"
@@ -41,6 +42,7 @@
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "net/base/escape.h"
+#include "net/base/filename_util.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -110,6 +112,17 @@ class DragAndDropSimulator {
     return SimulateDragEnter(location, *os_exchange_data_);
   }
 
+  // Simulates notification that |file| was dragged from outside of the browser,
+  // into the specified |location| inside |web_contents|.
+  // |location| is relative to |web_contents|.
+  // Returns true upon success.
+  bool SimulateDragEnter(const gfx::Point& location,
+                         const base::FilePath& file) {
+    os_exchange_data_ = std::make_unique<ui::OSExchangeData>();
+    os_exchange_data_->SetFilename(file);
+    return SimulateDragEnter(location, *os_exchange_data_);
+  }
+
   // Simulates dropping of the drag-and-dropped item.
   // SimulateDragEnter needs to be called first.
   // Returns true upon success.
@@ -686,6 +699,11 @@ class DragAndDropBrowserTest : public InProcessBrowserTest,
     return drag_simulator_->SimulateDragEnter(kMiddleOfRightFrame, url);
   }
 
+  bool SimulateDragEnterToRightFrame(const base::FilePath& file) {
+    AssertTestPageIsLoaded();
+    return drag_simulator_->SimulateDragEnter(kMiddleOfRightFrame, file);
+  }
+
   bool SimulateDropInRightFrame() {
     AssertTestPageIsLoaded();
     return drag_simulator_->SimulateDrop(kMiddleOfRightFrame);
@@ -817,10 +835,14 @@ IN_PROC_BROWSER_TEST_P(DragAndDropBrowserTest, MAYBE_DropTextFromOutside) {
 #else
 #define MAYBE_DropValidUrlFromOutside DropValidUrlFromOutside
 #endif
-// Scenario: drag URL from outside the browser and drop to the right frame.
-// Mostly focuses on covering 1) the navigation path, 2) focus behavior.  This
-// test explicitly does not cover the dragover and/or drop DOM events - they are
-// already covered via the DropTextFromOutside test above.
+// Scenario: drag URL from outside the browser and drop to the right frame
+// (e.g. this is similar to a drag that starts from the bookmarks bar, except
+// that here there is no drag start event - as-if the drag was started in
+// another application).
+//
+// This test mostly focuses on covering 1) the navigation path, 2) focus
+// behavior.  This test explicitly does not cover the dragover and/or drop DOM
+// events - they are already covered via the DropTextFromOutside test above.
 IN_PROC_BROWSER_TEST_P(DragAndDropBrowserTest, MAYBE_DropValidUrlFromOutside) {
   std::string frame_site = use_cross_site_subframe() ? "b.com" : "a.com";
   ASSERT_TRUE(NavigateToTestPage("a.com"));
@@ -846,19 +868,67 @@ IN_PROC_BROWSER_TEST_P(DragAndDropBrowserTest, MAYBE_DropValidUrlFromOutside) {
   // Drop into the right frame.
   ASSERT_TRUE(SimulateDropInRightFrame());
 
-  // Dropping |dragged_url| into:
-  // - a same-origin subframe - creates a new tab and navigates it to that URL.
-  // - a cross-origin subframe - presently does nothing (crbug.com/1087898).
-  if (!use_cross_site_subframe()) {
-    // Verify that a new tab was navigated to |dragged_url|.
-    wait_for_new_tab.Wait();
-    EXPECT_EQ(2, browser()->tab_strip_model()->count());
-    content::WebContents* new_web_contents =
-        browser()->tab_strip_model()->GetActiveWebContents();
-    content::TestNavigationObserver(new_web_contents, 1).Wait();
-    EXPECT_EQ(dragged_url,
-              new_web_contents->GetMainFrame()->GetLastCommittedURL());
-  }
+  // Verify that dropping |dragged_url| creates a new tab and navigates it to
+  // that URL.
+  wait_for_new_tab.Wait();
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+  content::WebContents* new_web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::TestNavigationObserver(new_web_contents, 1).Wait();
+  EXPECT_EQ(dragged_url,
+            new_web_contents->GetMainFrame()->GetLastCommittedURL());
+
+  // Verify that the initial tab didn't navigate.
+  EXPECT_EQ(initial_url, web_contents->GetMainFrame()->GetLastCommittedURL());
+  EXPECT_EQ(initial_history_count, controller.GetEntryCount());
+
+  // Verify that the focus moved from the omnibox to the tab contents.
+  EXPECT_FALSE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_TAB_CONTAINER));
+}
+
+// Scenario: drag a file from outside the browser and drop to the right frame
+// (e.g. starting a drag in a separate file explorer application, like Nemo on
+// gLinux).
+//
+// This test mostly focuses on covering 1) the navigation path, 2) focus
+// behavior.  This test explicitly does not cover the dragover and/or drop DOM
+// events - they are already covered via the DropTextFromOutside test above.
+IN_PROC_BROWSER_TEST_P(DragAndDropBrowserTest, DropFileFromOutside) {
+  std::string frame_site = use_cross_site_subframe() ? "b.com" : "a.com";
+  ASSERT_TRUE(NavigateToTestPage("a.com"));
+  ASSERT_TRUE(NavigateRightFrame(frame_site, "title1.html"));
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::NavigationController& controller = web_contents->GetController();
+  int initial_history_count = controller.GetEntryCount();
+  GURL initial_url = web_contents->GetMainFrame()->GetLastCommittedURL();
+  ASSERT_EQ(1, browser()->tab_strip_model()->count());
+
+  // Focus the omnibox.
+  chrome::FocusLocationBar(browser());
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_FALSE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_TAB_CONTAINER));
+
+  // Drag a file from outside the browser into/over the right frame.
+  base::FilePath dragged_file = ui_test_utils::GetTestFilePath(
+      base::FilePath(), base::FilePath().AppendASCII("title3.html"));
+  ASSERT_TRUE(SimulateDragEnterToRightFrame(dragged_file));
+
+  ui_test_utils::TabAddedWaiter wait_for_new_tab(browser());
+
+  // Drop into the right frame.
+  ASSERT_TRUE(SimulateDropInRightFrame());
+
+  // Verify that dropping |dragged_file| creates a new tab and navigates it to
+  // the corresponding file: URL.
+  wait_for_new_tab.Wait();
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+  content::WebContents* new_web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::TestNavigationObserver(new_web_contents, 1).Wait();
+  EXPECT_EQ(net::FilePathToFileURL(dragged_file),
+            new_web_contents->GetMainFrame()->GetLastCommittedURL());
 
   // Verify that the initial tab didn't navigate.
   EXPECT_EQ(initial_url, web_contents->GetMainFrame()->GetLastCommittedURL());

third_party/blink/renderer/core/frame/remote_frame.cc

@@ -131,6 +131,11 @@ void RemoteFrame::Trace(Visitor* visitor) const {
 
 void RemoteFrame::Navigate(FrameLoadRequest& frame_request,
                            WebFrameLoadType frame_load_type) {
+  // RemoteFrame::Navigate doesn't support policies like
+  // kNavigationPolicyNewForegroundTab - such policies need to be handled via
+  // local frames.
+  DCHECK_EQ(kNavigationPolicyCurrentTab, frame_request.GetNavigationPolicy());
+
   if (!navigation_rate_limiter().CanProceed())
     return;
 

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

@@ -307,7 +307,7 @@ void DragController::PerformDrag(DragData* drag_data, LocalFrame& local_root) {
       // current tab. See https://crbug.com/451659.
       request.SetNavigationPolicy(
           NavigationPolicy::kNavigationPolicyNewForegroundTab);
-      page_->MainFrame()->Navigate(request, WebFrameLoadType::kStandard);
+      local_root.Navigate(request, WebFrameLoadType::kStandard);
     }
 
     // TODO(bokan): This case happens when we end a URL drag inside a guest