Unconsumed file or URL drops into Blink should navigate in a new tab

Users can easily lose work if they drag/drop a file or URL into a tab
that does not consume the dropped data (e.g. as a file upload). Rather
than navigating the current tab (blowing away whatever was in it),
instead open the dropped URL in a new foreground tab.

Bug: 451659
Change-Id: Ic7010a3c933d7747d4b70fefa4d0a5380448c58c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2213459Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Eric Lawrence [MSFT] <ericlaw@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#774391}

chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc

@@ -829,6 +829,8 @@ IN_PROC_BROWSER_TEST_P(DragAndDropBrowserTest, MAYBE_DropValidUrlFromOutside) {
       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());
@@ -839,15 +841,28 @@ IN_PROC_BROWSER_TEST_P(DragAndDropBrowserTest, MAYBE_DropValidUrlFromOutside) {
   GURL dragged_url = embedded_test_server()->GetURL("d.com", "/title2.html");
   ASSERT_TRUE(SimulateDragEnterToRightFrame(dragged_url));
 
-  // Drop into the right frame - this should initiate navigating the main frame
-  // to |dragged_url|.
-  content::TestNavigationObserver nav_observer(web_contents, 1);
+  ui_test_utils::TabAddedWaiter wait_for_new_tab(browser());
+
+  // Drop into the right frame.
   ASSERT_TRUE(SimulateDropInRightFrame());
 
-  // Verify that the main frame got navigated to |dragged_url|.
-  nav_observer.Wait();
-  EXPECT_EQ(dragged_url, web_contents->GetMainFrame()->GetLastCommittedURL());
-  EXPECT_EQ(initial_history_count + 1, controller.GetEntryCount());
+  // 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 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));

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

@@ -301,6 +301,11 @@ void DragController::PerformDrag(DragData* drag_data, LocalFrame& local_root) {
       resource_request.SetRequestorOrigin(SecurityOrigin::CreateUniqueOpaque());
 
       FrameLoadRequest request(nullptr, resource_request);
+
+      // Open the dropped URL in a new tab to avoid potential data-loss in the
+      // current tab. See https://crbug.com/451659.
+      request.SetNavigationPolicy(
+          NavigationPolicy::kNavigationPolicyNewForegroundTab);
       page_->MainFrame()->Navigate(request, WebFrameLoadType::kStandard);
     }
 

third_party/blink/web_tests/editing/pasteboard/file-drag-to-editable-expected.txt

@@ -2,7 +2,7 @@ Tests that dragging a file into an editable area does not insert a filename.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
-PASS window.attemptedFileNavigation is true
+PASS window.attemptedSameTabFileNavigation is false
 PASS successfullyParsed is true
 
 TEST COMPLETE

third_party/blink/web_tests/editing/pasteboard/script-tests/file-drag-to-editable.js

@@ -5,7 +5,8 @@ editableArea.contentEditable = true;
 editableArea.style.background = 'green';
 editableArea.style.height = '100px';
 editableArea.style.width = '100px';
-// Important that we put this at the top of the doc so that logging does not cause it to go out of view (where it can't be dragged to)
+// Important that we put this at the top of the doc so that logging does not
+// cause it to go out of view (where it can't be dragged to).
 document.body.insertBefore(editableArea, document.body.firstChild);
 
 function moveMouseToCenterOfElement(element)
@@ -25,15 +26,18 @@ function dragFilesOntoEditableArea(files)
 
 function runTest()
 {
+    window.attemptedSameTabFileNavigation = false;
     window.onbeforeunload = function() {
-        window.attemptedFileNavigation = true;
+        window.attemptedSameTabFileNavigation = true;
 
-        // Don't remove the editable node, since we want to make sure there no stray file URLs were
-        // inserted during the drop.
+        // Don't remove the editable node, since we want to make sure no stray
+        // file URLs were inserted during the drop.
     };
     dragFilesOntoEditableArea(['DRTFakeFile']);
 
-    shouldBeTrue("window.attemptedFileNavigation");
+    // The file load should occur in a new tab (crbug.com/451659), so we
+    // should not attempt to navigate this tab.
+    shouldBeFalse("window.attemptedSameTabFileNavigation");
     finishJSTest();
 }
 

third_party/blink/web_tests/fast/events/drag-file-crash-expected.txt

-PASS
+ This is a test for https://bugs.webkit.org/show_bug.cgi?id=29276. It passes if it does not crash. If not run from DRT, drag a file onto the file input.

third_party/blink/web_tests/fast/events/drag-file-crash.html

@@ -9,26 +9,35 @@
         testRunner.waitUntilDone();
     }
 
-    function moveMouseToCenterOfElement(element)
-    {
-        var centerX = element.offsetLeft + element.offsetWidth / 2;
-        var centerY = element.offsetTop + element.offsetHeight / 2;
-        eventSender.mouseMoveTo(centerX, centerY);
-    }
+    const moveMouseToCenterOfElement = element => {
+        const clientRect = element.getBoundingClientRect();
+        const centerX = (clientRect.left + clientRect.right) / 2;
+        const centerY = (clientRect.top + clientRect.bottom) / 2;
+        if (window.eventSender)
+          eventSender.mouseMoveTo(centerX, centerY);
+    };
 
     function run()
     {
         window.scrollBy(0, 1000);
+        // Drop a file on a file input control.
         if (window.eventSender) {
-            eventSender.beginDragWithFiles(["resources/drag-file-crash-pass.html"]);
+            eventSender.beginDragWithFiles(["resources/abe.png"]);
             var fileInput = document.getElementById('file');
             moveMouseToCenterOfElement(fileInput);
             eventSender.mouseUp();
         }
     }
+
+    function fileDropped(t)
+    {
+        if (window.testRunner) {
+          testRunner.notifyDone();
+        }
+    }
 </script>
 <body onload="run()">
 <div id="scroller"></div>
-<input type="file" id="file">
+<input type="file" id="file" onchange="fileDropped(this)">
 This is a test for https://bugs.webkit.org/show_bug.cgi?id=29276. It passes if it does not crash. If not run from DRT, drag a file onto the file input.
 </body>

third_party/blink/web_tests/fast/events/drag-to-navigate-expected.txt

-PASS
+Begin drag
+PASS: Drag complete
+

third_party/blink/web_tests/fast/events/drag-to-navigate.html

-<div>FAIL</div>
+<!doctype html>
 <script>
+'use strict';
+function log(text)
+{
+    document.body.appendChild(document.createElement('div').appendChild(
+      document.createTextNode(text)));
+    document.body.appendChild(document.createElement('br'));
+}
 function doTest() {
-    testRunner.dumpAsText();
-
-    eventSender.beginDragWithFiles(["resources/file-for-drag-to-navigate.html"]);
+    log('Begin drag');
+    eventSender.beginDragWithFiles(
+      ["resources/file-for-drag-to-navigate.html"]);
     eventSender.mouseMoveTo(10, 10);
     eventSender.mouseUp();
+    log('PASS: Drag complete');
 }
-
 if (window.eventSender) {
+    testRunner.dumpAsText();
+    // notifyDone will be called by the new tab opened to the URL that is
+    // dropped above.
     testRunner.waitUntilDone();
-    // The load seems to fail if we try to kick of a
-    // new load before this one is finished.  So we wait.
-    window.onload = doTest();
+    window.onload = doTest;
 }
 </script>
+

third_party/blink/web_tests/fast/events/drop-handler-should-not-stop-navigate-expected.txt

-CONFIRM NAVIGATION
-This tests that a drop handler's default action must be prevented in order to stop navigation. Otherwise, if event.preventDefault() is not called, navigation should occur. To test manually, simply drag and drop another link or HTML file on this page. If navigation occurs, then the test passed.
+This tests that a drop handler's default action must be prevented in order to stop navigation. Otherwise, if event.preventDefault() is not called, navigation should occur. To test manually, simply drag and drop another link or HTML file on this page. If navigation of a new tab occurs, then the test passed.
 
 Starting test
 Not preventing default event on drop.
-PASS

third_party/blink/web_tests/fast/events/drop-handler-should-not-stop-navigate.html

@@ -3,20 +3,14 @@
 <body>
 <div>This tests that a drop handler's default action must be prevented in order to stop navigation.
 Otherwise, if event.preventDefault() is not called, navigation should occur. To test manually,
-simply drag and drop another link or HTML file on this page. If navigation occurs, then the test
-passed.</div>
+simply drag and drop another link or HTML file on this page. If navigation of a new tab occurs,
+then the test passed.</div>
 <script>
 function log(text)
 {
     document.body.appendChild(document.createElement('br'));
     document.body.appendChild(document.createElement('div').appendChild(document.createTextNode(text)));
 }
-window.addEventListener('beforeunload', function (e)
-{
-    log('PASS');
-    testRunner.notifyDone();
-    e.returnValue = "foo";
-});
 document.body.addEventListener('dragenter', function (event)
 {
     event.preventDefault();
@@ -38,16 +32,16 @@ document.body.addEventListener('drop', function (event)
     // The line below is necessary in order for preventing the default in
     // beforeunload handler to work properly. crbug.com/802982
     testRunner.setShouldStayOnPageAfterHandlingBeforeUnload(true);
+    window.addEventListener('beforeunload', function (e) {
+        log('FAIL: navigation should have occurred in new tab');
+        testRunner.notifyDone();
+        e.returnValue = "foo";
+    });
+
     log('Starting test');
-    eventSender.beginDragWithFiles(['DRTFakeFile']);
+    eventSender.beginDragWithFiles(["resources/notify-done.html"]);
     eventSender.mouseMoveTo(document.body.offsetLeft + 10, document.body.offsetTop + 10);
     eventSender.mouseUp();
-    window.stop();
-    window.setTimeout(function ()
-    {
-        // Deadman's switch so we don't need to wait for the test to timeout to fail.
-        testRunner.notifyDone();
-    }, 0);
 })();
 </script>
 </body>

third_party/blink/web_tests/fast/events/only-valid-drop-targets-receive-file-drop-expected.txt

-CONFIRM NAVIGATION
 To run this test manually, drag a file to one of the two boxes below.
 
 Dropping in drop target 1 should result in a drop event.
-Dropping in drop target 2 should NOT result in a drop event (page will navigate).
+Dropping in drop target 2 should NOT result in a drop event (new tab will navigate).
 Starting drag...
 Drop received in drop target 1
 Starting drag...
-Drop not received
 

third_party/blink/web_tests/fast/events/only-valid-drop-targets-receive-file-drop.html

@@ -7,8 +7,6 @@ div, span {
 };
 </style>
 <script>
-var dropReceived;
-var beforeUnloadReceived = false;
 function log(str)
 {
     var result = document.getElementById('result');
@@ -18,7 +16,7 @@ function log(str)
 function dragDrop(target)
 {
     log('Starting drag...');
-    eventSender.beginDragWithFiles(['test']);
+    eventSender.beginDragWithFiles(['resources/notify-done.html']);
     eventSender.leapForward(100);
     eventSender.mouseMoveTo(target.offsetLeft + target.offsetWidth / 2,
                             target.offsetTop + target.offsetHeight / 2);
@@ -30,7 +28,6 @@ window.onload = function()
     var drop2 = document.getElementById('drop2');
 
     document.body.addEventListener('dragover', function () {
-        dropReceived = false;
     }, false);
 
     drop1.addEventListener('dragover', function(e) {
@@ -38,7 +35,6 @@ window.onload = function()
     }, false);
     drop1.addEventListener('drop', function(e) {
         e.preventDefault();
-        dropReceived = true;
         log('Drop received in drop target 1');
     }, false);
 
@@ -46,8 +42,7 @@ window.onload = function()
     }, false);
     drop2.addEventListener('drop', function(e) {
         e.preventDefault();
-        dropReceived = true;
-        log('Drop received in drop target 2');
+        log('FAIL: Drop unexpectedly received in drop target 2');
     }, false);
 
     if (!window.testRunner)
@@ -59,29 +54,20 @@ window.onload = function()
     testRunner.setShouldStayOnPageAfterHandlingBeforeUnload(true);
 
     window.addEventListener('beforeunload', function (e) {
-        if (!dropReceived)
-            log('Drop not received');
-        beforeUnloadReceived = true;
-        e.returnValue = "foo";
+        log('FAIL: navigation should have occurred in new tab');
         testRunner.notifyDone();
+        e.returnValue = "foo";
     });
 
     dragDrop(drop1);
     dragDrop(drop2);
-    window.stop();
-    window.setTimeout(function () {
-        // Deadman's switch to fail quickly.
-        if (!beforeUnloadReceived)
-            log('FAIL: navigation expected');
-        testRunner.notifyDone();
-    }, 0);
 }
 </script>
 </head>
 <body>
 <p>To run this test manually, drag a file to one of the two boxes below.
 <div id="drop1">Dropping in drop target 1 should result in a drop event.</div>
-<div id="drop2">Dropping in drop target 2 should NOT result in a drop event (page will navigate).</div>
+<div id="drop2">Dropping in drop target 2 should NOT result in a drop event (new tab will navigate).</div>
 <div id="result">
 </div>
 </body>

third_party/blink/web_tests/fast/events/resources/drag-file-crash-pass.html

-<!doctype html>
-PASS
-<script>
-    if (window.testRunner) {
-        testRunner.notifyDone();
-    }
-</script>

third_party/blink/web_tests/fast/events/resources/file-for-drag-to-navigate.html

-PASS
+FAIL: This page should have loaded in a new tab.
 <script>
 testRunner.notifyDone();
 </script>

third_party/blink/web_tests/http/tests/misc/drag-bookmark-expected.txt

-This is drag-bookmark-destination.html.
+PASS: Dropped bookmark

third_party/blink/web_tests/http/tests/misc/drag-bookmark.html

 <!DOCTYPE html>
 <script>
+'use strict';
+window.onload = () => {
+  // Simulate dragging a bookmark to a cross-site URL.
+  const crossSiteURL = "http://localhost:8000/misc/resources/" +
+                      "notify-success.html";
+  eventSender.beginDragWithStringData(crossSiteURL, "text/uri-list");
+  eventSender.mouseMoveTo(10, 10);
+  eventSender.mouseUp();
+  document.body.appendChild(document.createTextNode('PASS: Dropped bookmark'));
 
-// Simulate dragging a bookmark to a cross-site URL.
-const crossSiteURL = "http://localhost:8000/misc/resources/" +
-                     "drag-bookmark-destination.html";
-eventSender.beginDragWithStringData(crossSiteURL, "text/uri-list");
-eventSender.mouseMoveTo(10, 10);
-eventSender.mouseUp();
-
-// notifyDone will be called once we navigate to the "bookmarked" URL that is
-// being dragged above.
-testRunner.waitUntilDone();
-
+  testRunner.dumpAsText();
+  // notifyDone will be called once we navigate to the "bookmarked" URL that
+  // is being dragged above.
+  testRunner.waitUntilDone();
+};
 </script>

third_party/blink/web_tests/http/tests/misc/resources/drag-bookmark-destination.html

-<!DOCTYPE html>
-<script>
-testRunner.dumpAsText();
-</script>
-<body onload="testRunner.notifyDone()">
-  This is drag-bookmark-destination.html.
-</body>