Links in PDF should open in a new window when shift + left clicked.

chrome/test/data/pdf/navigator_test.js

Index: chrome/test/data/pdf/navigator_test.js
diff --git a/chrome/test/data/pdf/navigator_test.js b/chrome/test/data/pdf/navigator_test.js
index eb53a363ecdb793e273e3855ef8e45318fa17500..2c2f2c6e7e062b606ce266cac4bc146971a71f4e 100644
--- a/chrome/test/data/pdf/navigator_test.js
+++ b/chrome/test/data/pdf/navigator_test.js
@@ -2,20 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-function NavigateInCurrentTabCallback() {
-  this.navigateCalled = false;
-  this.url = undefined;
-  this.callback = function(url) {
-    this.navigateCalled = true;
-    this.url = url;
-  }.bind(this);
-  this.reset = function() {
-    this.navigateCalled = false;
-    this.url = undefined;
-  };
-}
-
-function NavigateInNewTabCallback() {
+function NavigateCallback() {

[raymes, 2016/09/06 04:36:07]

We would make this into a MockNavigatorDelegate class too. The state could then
be class-level state. We would need 3 separate bools to track whether each
function had been called.

[jaepark, 2016/09/06 20:32:47]

Done.

   this.navigateCalled = false;
   this.url = undefined;
   this.callback = function(url) {
@@ -29,10 +16,10 @@ function NavigateInNewTabCallback() {
 }
 
 /**
- * Given a |navigator|, navigate to |url| in the current tab or new tab,
- * depending on the value of |openInNewTab|. Use |viewportChangedCallback|
- * and |navigateCallback| to check the callbacks, and that the navigation
- * to |expectedResultUrl| happened.
+ * Given a |navigator|, navigate to |url| in the current tab, a new tab, or
+ * a new window depending on the value of |disposition|.
+ * Use |viewportChangedCallback| and |navigateCallback| to check the callbacks,
+ * and that the navigation to |expectedResultUrl| happened.
  */
 function doNavigationUrlTest(
     navigator,
@@ -50,22 +37,42 @@ function doNavigationUrlTest(
 }
 
 /**
- * Helper function to run doNavigationUrlTest() for the current tab and a new
- * tab.
+ * Helper function to run doNavigationUrlTest() for the current tab, a new
+ * tab, and a new window.
  */
-function doNavigationUrlTestInCurrentTabAndNewTab(
-    navigator,
-    url,
-    expectedResultUrl,
-    viewportChangedCallback,
-    navigateInCurrentTabCallback,
-    navigateInNewTabCallback) {
+function doNavigationUrlTests(originalUrl, url, expectedResultUrl) {
+  var mockWindow = new MockWindow(100, 100);
+  var mockSizer = new MockSizer();
+  var mockViewportChangedCallback = new MockViewportChangedCallback();
+  var viewport = new Viewport(mockWindow, mockSizer,
+                              mockViewportChangedCallback.callback,
+                              function() {}, function() {}, 0, 1, 0);
+
+  var paramsParser = new OpenPDFParamsParser(function(name) {
+    paramsParser.onNamedDestinationReceived(-1);
+  });
+
+  var navigateInCurrentTabCallback = new NavigateCallback();
+  var navigateInNewTabCallback = new NavigateCallback();
+  var navigateInNewWindowCallback = new NavigateCallback();
+
+  var navigateCallback = {
+    currentTabCallback: navigateInCurrentTabCallback.callback,
+    newTabCallback: navigateInNewTabCallback.callback,
+    newWindowCallback: navigateInNewWindowCallback.callback
+  };
+  var navigator = new Navigator(originalUrl, viewport, paramsParser,
+                                navigateCallback);

[raymes, 2016/09/06 04:36:07]

We could just pass in a new MockNavigatorDelegate here (and below)

[jaepark, 2016/09/06 20:32:47]

Done.

+
   doNavigationUrlTest(navigator, url,
       Navigator.WindowOpenDisposition.CURRENT_TAB, expectedResultUrl,
-      viewportChangedCallback, navigateInCurrentTabCallback);
+      mockViewportChangedCallback, navigateInCurrentTabCallback);

[raymes, 2016/09/06 04:36:07]

We could pass in the appropriate state of the MockNavigatorDelegate to check it
in each of these function calls.

Ideally we would check that only the correct function was called (and not
others). This would require a little bit of reworking of the test.

[jaepark, 2016/09/06 20:32:47]

Done.

   doNavigationUrlTest(navigator, url,
       Navigator.WindowOpenDisposition.NEW_BACKGROUND_TAB, expectedResultUrl,
-      viewportChangedCallback, navigateInNewTabCallback);
+      mockViewportChangedCallback, navigateInNewTabCallback);
+  doNavigationUrlTest(navigator, url,
+      Navigator.WindowOpenDisposition.NEW_WINDOW, expectedResultUrl,
+      mockViewportChangedCallback, navigateInNewWindowCallback);
 }
 
 var tests = [
@@ -90,11 +97,17 @@ var tests = [
     });
     var url = "http://xyz.pdf";
 
-    var navigateInCurrentTabCallback = new NavigateInCurrentTabCallback();
-    var navigateInNewTabCallback = new NavigateInNewTabCallback();
+    var navigateInCurrentTabCallback = new NavigateCallback();
+    var navigateInNewTabCallback = new NavigateCallback();
+    var navigateInNewWindowCallback = new NavigateCallback();
+
+    var navigateCallback = {
+      currentTabCallback: navigateInCurrentTabCallback.callback,
+      newTabCallback: navigateInNewTabCallback.callback,
+      newWindowCallback: navigateInNewWindowCallback.callback
+    };
     var navigator = new Navigator(url, viewport, paramsParser,
-        navigateInCurrentTabCallback.callback,
-        navigateInNewTabCallback.callback);
+                                  navigateCallback);
 
     var documentDimensions = new MockDocumentDimensions();
     documentDimensions.addPage(100, 100);
@@ -145,153 +158,87 @@ var tests = [
     chrome.test.succeed();
   },
   /**
-   * Test opening a url in the same tab and opening a url in a new tab for
+   * Test opening a url in the same tab, in a new tab, and in a new window for
    * a http:// url as the current location. The destination url may not have
    * a valid scheme, so the navigator must determine the url by following
    * similar heuristics as Adobe Acrobat Reader.
    */
   function testNavigateForLinksWithoutScheme() {
-    var mockWindow = new MockWindow(100, 100);
-    var mockSizer = new MockSizer();
-    var mockCallback = new MockViewportChangedCallback();
-    var viewport = new Viewport(mockWindow, mockSizer, mockCallback.callback,
-                                function() {}, function() {}, 0, 1, 0);
-
-    var paramsParser = new OpenPDFParamsParser(function(name) {
-      paramsParser.onNamedDestinationReceived(-1);
-    });
     var url = "http://www.example.com/subdir/xyz.pdf";
 
-    var navigateInCurrentTabCallback = new NavigateInCurrentTabCallback();
-    var navigateInNewTabCallback = new NavigateInNewTabCallback();
-    var navigator = new Navigator(url, viewport, paramsParser,
-        navigateInCurrentTabCallback.callback,
-        navigateInNewTabCallback.callback);
-
     // Sanity check.
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+    doNavigationUrlTests(
+        url,
         'https://www.foo.com/bar.pdf',
-        'https://www.foo.com/bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
+        'https://www.foo.com/bar.pdf');
 
     // Open relative links.
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+    doNavigationUrlTests(
+        url,
         'foo/bar.pdf',
-        'http://www.example.com/subdir/foo/bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+        'http://www.example.com/subdir/foo/bar.pdf');
+    doNavigationUrlTests(
+        url,
         'foo.com/bar.pdf',
-        'http://www.example.com/subdir/foo.com/bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
+        'http://www.example.com/subdir/foo.com/bar.pdf');
     // The expected result is not normalized here.
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+    doNavigationUrlTests(
+        url,
         '../www.foo.com/bar.pdf',
-        'http://www.example.com/subdir/../www.foo.com/bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
+        'http://www.example.com/subdir/../www.foo.com/bar.pdf');
 
     // Open an absolute link.
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+    doNavigationUrlTests(
+        url,
         '/foodotcom/bar.pdf',
-        'http://www.example.com/foodotcom/bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
+        'http://www.example.com/foodotcom/bar.pdf');
 
     // Open a http url without a scheme.
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+    doNavigationUrlTests(
+        url,
         'www.foo.com/bar.pdf',
-        'http://www.foo.com/bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
+        'http://www.foo.com/bar.pdf');
 
     // Test three dots.
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+    doNavigationUrlTests(
+        url,
         '.../bar.pdf',
-        'http://www.example.com/subdir/.../bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
+        'http://www.example.com/subdir/.../bar.pdf');
 
     // Test forward slashes.
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+    doNavigationUrlTests(
+        url,
         '..\\bar.pdf',
-        'http://www.example.com/subdir/..\\bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+        'http://www.example.com/subdir/..\\bar.pdf');
+    doNavigationUrlTests(
+        url,
         '.\\bar.pdf',
-        'http://www.example.com/subdir/.\\bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+        'http://www.example.com/subdir/.\\bar.pdf');
+    doNavigationUrlTests(
+        url,
         '\\bar.pdf',
-        'http://www.example.com/subdir/\\bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
+        'http://www.example.com/subdir/\\bar.pdf');
 
     // Regression test for https://crbug.com/569040
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
-        'http://something.else/foo#page=5',
+    doNavigationUrlTests(
+        url,
         'http://something.else/foo#page=5',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
+        'http://something.else/foo#page=5');
 
     chrome.test.succeed();
   },
   /**
-   * Test opening a url in the same tab and opening a url in a new tab with a
-   * file:/// url as the current location.
+   * Test opening a url in the same tab, in a new tab, and in a new window with
+   * a file:/// url as the current location.
    */
   function testNavigateFromLocalFile() {
-    var mockWindow = new MockWindow(100, 100);
-    var mockSizer = new MockSizer();
-    var mockCallback = new MockViewportChangedCallback();
-    var viewport = new Viewport(mockWindow, mockSizer, mockCallback.callback,
-                                function() {}, function() {}, 0, 1, 0);
-
-    var paramsParser = new OpenPDFParamsParser(function(name) {
-      paramsParser.onNamedDestinationReceived(-1);
-    });
     var url = "file:///some/path/to/myfile.pdf";
 
-    var navigateInCurrentTabCallback = new NavigateInCurrentTabCallback();
-    var navigateInNewTabCallback = new NavigateInNewTabCallback();
-    var navigator = new Navigator(url, viewport, paramsParser,
-        navigateInCurrentTabCallback.callback,
-        navigateInNewTabCallback.callback);
-
     // Open an absolute link.
-    doNavigationUrlTestInCurrentTabAndNewTab(
-        navigator,
+    doNavigationUrlTests(
+        url,
         '/foodotcom/bar.pdf',
-        'file:///foodotcom/bar.pdf',
-        mockCallback,
-        navigateInCurrentTabCallback,
-        navigateInNewTabCallback);
+        'file:///foodotcom/bar.pdf');
 
     chrome.test.succeed();
   }