Handle ctrl + shift + left click on links in PDF.

Ctrl + shift + left (or shift + middle) click on links should open a new
foreground tab.

BUG=630075
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2166193002
Cr-Commit-Position: refs/heads/master@{#407840}

chrome/browser/pdf/pdf_extension_test.cc

@@ -60,6 +60,12 @@
 
 const int kNumberLoadTestParts = 10;
 
+#if defined(OS_MACOSX)
+const int kDefaultKeyModifier = blink::WebInputEvent::MetaKey;
+#else
+const int kDefaultKeyModifier = blink::WebInputEvent::ControlKey;
+#endif
+
 // Using ASSERT_TRUE deliberately instead of ASSERT_EQ or ASSERT_STREQ
 // in order to print a more readable message if the strings differ.
 #define ASSERT_MULTILINE_STREQ(expected, actual) \
@@ -715,16 +721,10 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest, LinkCtrlLeftClick) {
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
 
-#if defined(OS_MACOSX)
-  int modifiers = blink::WebInputEvent::MetaKey;
-#else
-  int modifiers = blink::WebInputEvent::ControlKey;
-#endif
-
   content::WindowedNotificationObserver observer(
       chrome::NOTIFICATION_TAB_ADDED,
       content::NotificationService::AllSources());
-  content::SimulateMouseClickAt(web_contents, modifiers,
+  content::SimulateMouseClickAt(web_contents, kDefaultKeyModifier,
       blink::WebMouseEvent::ButtonLeft, link_position);
   observer.Wait();
 
@@ -780,3 +780,69 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest, LinkMiddleClick) {
   const GURL& url = new_web_contents->GetURL();
   ASSERT_EQ(std::string("http://www.example.com/"), url.spec());
 }
+
+IN_PROC_BROWSER_TEST_F(PDFExtensionTest, LinkCtrlShiftLeftClick) {
+  host_resolver()->AddRule("www.example.com", "127.0.0.1");
+  GURL test_pdf_url(embedded_test_server()->GetURL("/pdf/test-link.pdf"));
+  content::WebContents* guest_contents = LoadPdfGetGuestContents(test_pdf_url);
+  ASSERT_TRUE(guest_contents);
+
+  // The link position of the test-link.pdf in page coordinates is (110, 110).
+  // Convert the link position from page coordinates to screen coordinates.
+  gfx::Point link_position(110, 110);
+  ConvertPageCoordToScreenCoord(guest_contents, &link_position);
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  int modifiers = blink::WebInputEvent::ShiftKey | kDefaultKeyModifier;
+
+  content::WindowedNotificationObserver observer(
+      chrome::NOTIFICATION_TAB_ADDED,
+      content::NotificationService::AllSources());
+  content::SimulateMouseClickAt(web_contents, modifiers,
+      blink::WebMouseEvent::ButtonLeft, link_position);
+  observer.Wait();
+
+  int tab_count = browser()->tab_strip_model()->count();
+  ASSERT_EQ(2, tab_count);
+
+  content::WebContents* active_web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_NE(web_contents, active_web_contents);
+
+  const GURL& url = active_web_contents->GetURL();
+  ASSERT_EQ(std::string("http://www.example.com/"), url.spec());
+}
+
+IN_PROC_BROWSER_TEST_F(PDFExtensionTest, LinkShiftMiddleClick) {
+  host_resolver()->AddRule("www.example.com", "127.0.0.1");
+  GURL test_pdf_url(embedded_test_server()->GetURL("/pdf/test-link.pdf"));
+  content::WebContents* guest_contents = LoadPdfGetGuestContents(test_pdf_url);
+  ASSERT_TRUE(guest_contents);
+
+  // The link position of the test-link.pdf in page coordinates is (110, 110).
+  // Convert the link position from page coordinates to screen coordinates.
+  gfx::Point link_position(110, 110);
+  ConvertPageCoordToScreenCoord(guest_contents, &link_position);
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  content::WindowedNotificationObserver observer(
+      chrome::NOTIFICATION_TAB_ADDED,
+      content::NotificationService::AllSources());
+  content::SimulateMouseClickAt(web_contents, blink::WebInputEvent::ShiftKey,
+      blink::WebMouseEvent::ButtonMiddle, link_position);
+  observer.Wait();
+
+  int tab_count = browser()->tab_strip_model()->count();
+  ASSERT_EQ(2, tab_count);
+
+  content::WebContents* active_web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_NE(web_contents, active_web_contents);
+
+  const GURL& url = active_web_contents->GetURL();
+  ASSERT_EQ(std::string("http://www.example.com/"), url.spec());
+}

chrome/browser/resources/pdf/navigator.js

@@ -11,31 +11,45 @@
  * @param {Object} paramsParser The object for URL parsing.
  * @param {Function} navigateInCurrentTabCallback The Callback function that
  *    gets called when navigation happens in the current tab.
- * @param {Function} navigateInNewBackgroundTabCallback The Callback function
- *    that gets called when navigation happens in the new background tab.
+ * @param {Function} navigateInNewTabCallback The Callback function
+ *    that gets called when navigation happens in the new tab.
  */
 function Navigator(originalUrl,
                    viewport,
                    paramsParser,
                    navigateInCurrentTabCallback,
-                   navigateInNewBackgroundTabCallback) {
+                   navigateInNewTabCallback) {
   this.originalUrl_ = originalUrl;
   this.viewport_ = viewport;
   this.paramsParser_ = paramsParser;
   this.navigateInCurrentTabCallback_ = navigateInCurrentTabCallback;
-  this.navigateInNewBackgroundTabCallback_ = navigateInNewBackgroundTabCallback;
+  this.navigateInNewTabCallback_ = navigateInNewTabCallback;
 }
 
+/**
+ * Represents options when navigating to a new url. C++ counterpart of
+ * the enum is in ui/base/window_open_disposition.h. This enum represents
+ * the only values that are passed from Plugin.
+ * @enum {number}
+ */
+Navigator.WindowOpenDisposition = {
+  CURRENT_TAB: 1,
+  NEW_FOREGROUND_TAB: 3,
+  NEW_BACKGROUND_TAB: 4,
+  NEW_WINDOW: 6,
+  SAVE_TO_DISK: 7
+};
+
 Navigator.prototype = {
   /**
    * @private
    * Function to navigate to the given URL. This might involve navigating
    * within the PDF page or opening a new url (in the same tab or a new tab).
    * @param {string} url The URL to navigate to.
-   * @param {boolean} newTab Whether to perform the navigation in a new tab or
-   *    in the current tab.
+   * @param {number} disposition The window open disposition when
+   *    navigating to the new URL.
    */
-  navigate: function(url, newTab) {
+  navigate: function(url, disposition) {
     if (url.length == 0)
       return;
 
@@ -58,11 +72,31 @@ Navigator.prototype = {
     if (!this.isValidUrl_(url))
       return;
 
-    if (newTab) {
-      this.navigateInNewBackgroundTabCallback_(url);
-    } else {
-      this.paramsParser_.getViewportFromUrlParams(
-          url, this.onViewportReceived_.bind(this));
+    switch (disposition) {
+      case Navigator.WindowOpenDisposition.CURRENT_TAB:
+        this.paramsParser_.getViewportFromUrlParams(
+            url, this.onViewportReceived_.bind(this));
+        break;
+      case Navigator.WindowOpenDisposition.NEW_BACKGROUND_TAB:
+        this.navigateInNewTabCallback_(url, false);
+        break;
+      case Navigator.WindowOpenDisposition.NEW_FOREGROUND_TAB:
+        this.navigateInNewTabCallback_(url, true);
+        break;
+      case Navigator.WindowOpenDisposition.NEW_WINDOW:
+        // TODO(jaepark): Shift + left clicking a link in PDF should open the
+        // link in a new window. See http://crbug.com/628057.
+        this.paramsParser_.getViewportFromUrlParams(
+            url, this.onViewportReceived_.bind(this));
+        break;
+      case Navigator.WindowOpenDisposition.SAVE_TO_DISK:
+        // TODO(jaepark): Alt + left clicking a link in PDF should
+        // download the link.
+        this.paramsParser_.getViewportFromUrlParams(
+            url, this.onViewportReceived_.bind(this));
+        break;
+      default:
+        break;
     }
   },
 

chrome/browser/resources/pdf/pdf.js

@@ -56,14 +56,15 @@ function onNavigateInCurrentTab(isInTab, isSourceFileUrl, url) {
 }
 
 /**
- * Called when navigation happens in the new background tab.
- * @param {string} url The url to be opened in the new background tab.
+ * Called when navigation happens in the new tab.
+ * @param {string} url The url to be opened in the new tab.
+ * @param {boolean} active Indicates if the new tab should be the active tab.
  */
-function onNavigateInNewBackgroundTab(url) {
+function onNavigateInNewTab(url, active) {
   // Prefer the tabs API because it guarantees we can just open a new tab.
   // window.open doesn't have this guarantee.
   if (chrome.tabs)
-    chrome.tabs.create({url: url, active: false});
+    chrome.tabs.create({url: url, active: active});
   else
     window.open(url);
 }
@@ -238,7 +239,10 @@ function PDFViewer(browserApi) {
   }.bind(this));
 
   document.body.addEventListener('navigate', function(e) {
-    this.navigator_.navigate(e.detail.uri, e.detail.newtab);
+    var disposition =
+        e.detail.newtab ? Navigator.WindowOpenDisposition.NEW_BACKGROUND_TAB :
+                          Navigator.WindowOpenDisposition.CURRENT_TAB;
+    this.navigator_.navigate(e.detail.uri, disposition);
   }.bind(this));
 
   this.toolbarManager_ =
@@ -264,7 +268,7 @@ function PDFViewer(browserApi) {
                                   onNavigateInCurrentTab.bind(undefined,
                                                               isInTab,
                                                               isSourceFileUrl),
-                                  onNavigateInNewBackgroundTab);
+                                  onNavigateInNewTab);
   this.viewportScroller_ =
       new ViewportScroller(this.viewport_, this.plugin_, window);
 
@@ -627,10 +631,13 @@ PDFViewer.prototype = {
         break;
       case 'navigate':
         // If in print preview, always open a new tab.
-        if (this.isPrintPreview_)
-          this.navigator_.navigate(message.data.url, true);
-        else
-          this.navigator_.navigate(message.data.url, message.data.newTab);
+        if (this.isPrintPreview_) {
+          this.navigator_.navigate(
+              message.data.url,
+              Navigator.WindowOpenDisposition.NEW_BACKGROUND_TAB);
+        } else {
+          this.navigator_.navigate(message.data.url, message.data.disposition);
+        }
         break;
       case 'setScrollPosition':
         var position = this.viewport_.position;

chrome/test/data/pdf/navigator_test.js

@@ -37,13 +37,13 @@ function NavigateInNewTabCallback() {
 function doNavigationUrlTest(
     navigator,
     url,
-    openInNewTab,
+    disposition,
     expectedResultUrl,
     viewportChangedCallback,
     navigateCallback) {
   viewportChangedCallback.reset();
   navigateCallback.reset();
-  navigator.navigate(url, openInNewTab);
+  navigator.navigate(url, disposition);
   chrome.test.assertFalse(viewportChangedCallback.wasCalled);
   chrome.test.assertTrue(navigateCallback.navigateCalled);
   chrome.test.assertEq(expectedResultUrl, navigateCallback.url);
@@ -60,9 +60,11 @@ function doNavigationUrlTestInCurrentTabAndNewTab(
     viewportChangedCallback,
     navigateInCurrentTabCallback,
     navigateInNewTabCallback) {
-  doNavigationUrlTest(navigator, url, false, expectedResultUrl,
+  doNavigationUrlTest(navigator, url,
+      Navigator.WindowOpenDisposition.CURRENT_TAB, expectedResultUrl,
       viewportChangedCallback, navigateInCurrentTabCallback);
-  doNavigationUrlTest(navigator, url, true, expectedResultUrl,
+  doNavigationUrlTest(navigator, url,
+      Navigator.WindowOpenDisposition.NEW_BACKGROUND_TAB, expectedResultUrl,
       viewportChangedCallback, navigateInNewTabCallback);
 }
 
@@ -103,7 +105,8 @@ var tests = [
 
     mockCallback.reset();
     // This should move viewport to page 0.
-    navigator.navigate(url + "#US", false);
+    navigator.navigate(url + "#US",
+        Navigator.WindowOpenDisposition.CURRENT_TAB);
     chrome.test.assertTrue(mockCallback.wasCalled);
     chrome.test.assertEq(0, viewport.position.x);
     chrome.test.assertEq(0, viewport.position.y);
@@ -112,7 +115,8 @@ var tests = [
     navigateInNewTabCallback.reset();
     // This should open "http://xyz.pdf#US" in a new tab. So current tab
     // viewport should not update and viewport position should remain same.
-    navigator.navigate(url + "#US", true);
+    navigator.navigate(url + "#US",
+        Navigator.WindowOpenDisposition.NEW_BACKGROUND_TAB);
     chrome.test.assertFalse(mockCallback.wasCalled);
     chrome.test.assertTrue(navigateInNewTabCallback.navigateCalled);
     chrome.test.assertEq(0, viewport.position.x);
@@ -120,7 +124,8 @@ var tests = [
 
     mockCallback.reset();
     // This should move viewport to page 2.
-    navigator.navigate(url + "#UY", false);
+    navigator.navigate(url + "#UY",
+        Navigator.WindowOpenDisposition.CURRENT_TAB);
     chrome.test.assertTrue(mockCallback.wasCalled);
     chrome.test.assertEq(0, viewport.position.x);
     chrome.test.assertEq(300, viewport.position.y);
@@ -130,7 +135,8 @@ var tests = [
     // #ABC is not a named destination in the page so viewport should not
     // update and viewport position should remain same. As this link will open
     // in the same tab.
-    navigator.navigate(url + "#ABC", false);
+    navigator.navigate(url + "#ABC",
+        Navigator.WindowOpenDisposition.CURRENT_TAB);
     chrome.test.assertFalse(mockCallback.wasCalled);
     chrome.test.assertTrue(navigateInCurrentTabCallback.navigateCalled);
     chrome.test.assertEq(0, viewport.position.x);

pdf/BUILD.gn

@@ -16,6 +16,7 @@ static_library("pdf") {
     "//net",
     "//ppapi/cpp:objects",
     "//ppapi/cpp/private:internal_module",
+    "//ui/base",
   ]
 
   sources = [

pdf/DEPS

@@ -2,6 +2,7 @@ include_rules = [
   "+chrome/common/content_restriction.h",
   "+net",
   "+ppapi",
+  "+ui/base/window_open_disposition.h",
   "+ui/events/keycodes/keyboard_codes.h",
   "+v8/include/v8.h"
 ]

pdf/out_of_process_instance.cc

@@ -104,7 +104,7 @@ const char kJSCancelStreamUrlType[] = "cancelStreamUrl";
 // Navigate to the given URL (Plugin -> Page)
 const char kJSNavigateType[] = "navigate";
 const char kJSNavigateUrl[] = "url";
-const char kJSNavigateNewTab[] = "newTab";
+const char kJSNavigateWindowOpenDisposition[] = "disposition";
 // Open the email editor with the given parameters (Plugin -> Page)
 const char kJSEmailType[] = "email";
 const char kJSEmailTo[] = "to";
@@ -977,11 +977,11 @@ void OutOfProcessInstance::ScrollToPage(int page) {
 }
 
 void OutOfProcessInstance::NavigateTo(const std::string& url,
-                                      bool open_in_new_tab) {
+                                      WindowOpenDisposition disposition) {
   pp::VarDictionary message;
   message.Set(kType, kJSNavigateType);
   message.Set(kJSNavigateUrl, url);
-  message.Set(kJSNavigateNewTab, open_in_new_tab);
+  message.Set(kJSNavigateWindowOpenDisposition, pp::Var(disposition));
   PostMessage(message);
 }
 

pdf/out_of_process_instance.h

@@ -102,7 +102,8 @@ class OutOfProcessInstance : public pp::Instance,
   void ScrollToX(int position) override;
   void ScrollToY(int position) override;
   void ScrollToPage(int page) override;
-  void NavigateTo(const std::string& url, bool open_in_new_tab) override;
+  void NavigateTo(const std::string& url,
+                  WindowOpenDisposition disposition) override;
   void UpdateCursor(PP_CursorType_Dev cursor) override;
   void UpdateTickMarks(const std::vector<pp::Rect>& tickmarks) override;
   void NotifyNumberOfFindResultsChanged(int total, bool final_result) override;

pdf/pdf.gyp

@@ -14,6 +14,7 @@
         '../net/net.gyp:net',
         '../ppapi/ppapi.gyp:ppapi_cpp_objects',
         '../ppapi/ppapi.gyp:ppapi_internal_module',
+        '../ui/base/ui_base.gyp:ui_base',
       ],
       'ldflags': [ '-L<(PRODUCT_DIR)',],
       'sources': [

pdf/pdf_engine.h

@@ -27,6 +27,7 @@
 #include "ppapi/cpp/size.h"
 #include "ppapi/cpp/url_loader.h"
 #include "ppapi/cpp/var_array.h"
+#include "ui/base/window_open_disposition.h"
 
 namespace pp {
 class InputEvent;
@@ -37,12 +38,6 @@ namespace chrome_pdf {
 
 class Stream;
 
-#if defined(OS_MACOSX)
-const uint32_t kDefaultKeyModifier = PP_INPUTEVENT_MODIFIER_METAKEY;
-#else  // !OS_MACOSX
-const uint32_t kDefaultKeyModifier = PP_INPUTEVENT_MODIFIER_CONTROLKEY;
-#endif  // OS_MACOSX
-
 // Do one time initialization of the SDK.
 bool InitializeSDK();
 // Tells the SDK that we're shutting down.
@@ -80,7 +75,8 @@ class PDFEngine {
     virtual void ScrollToPage(int page) = 0;
 
     // Navigate to the given url.
-    virtual void NavigateTo(const std::string& url, bool open_in_new_tab) = 0;
+    virtual void NavigateTo(const std::string& url,
+                            WindowOpenDisposition disposition) = 0;
 
     // Updates the cursor.
     virtual void UpdateCursor(PP_CursorType_Dev cursor) = 0;

pdf/pdfium/pdfium_engine.cc

@@ -1643,9 +1643,19 @@ bool PDFiumEngine::OnMouseUp(const pp::MouseInputEvent& event) {
   // Open link on mouse up for same link for which mouse down happened earlier.
   if (mouse_down_state_.Matches(area, target)) {
     if (area == PDFiumPage::WEBLINK_AREA) {
-      bool open_in_new_tab = !!(event.GetModifiers() & kDefaultKeyModifier) ||
-          !!(event.GetModifiers() & PP_INPUTEVENT_MODIFIER_MIDDLEBUTTONDOWN);
-      client_->NavigateTo(target.url, open_in_new_tab);
+      uint32_t modifiers = event.GetModifiers();
+      bool middle_button =
+          !!(modifiers & PP_INPUTEVENT_MODIFIER_MIDDLEBUTTONDOWN);
+      bool alt_key = !!(modifiers & PP_INPUTEVENT_MODIFIER_ALTKEY);
+      bool ctrl_key = !!(modifiers & PP_INPUTEVENT_MODIFIER_CONTROLKEY);
+      bool meta_key = !!(modifiers & PP_INPUTEVENT_MODIFIER_METAKEY);
+      bool shift_key = !!(modifiers & PP_INPUTEVENT_MODIFIER_SHIFTKEY);
+
+      WindowOpenDisposition disposition =
+          ui::DispositionFromClick(middle_button, alt_key, ctrl_key, meta_key,
+                                   shift_key);
+
+      client_->NavigateTo(target.url, disposition);
       client_->FormTextFieldFocusChange(false);
       return true;
     }
@@ -3524,7 +3534,7 @@ void PDFiumEngine::Form_SetTextFieldFocus(FPDF_FORMFILLINFO* param,
 void PDFiumEngine::Form_DoURIAction(FPDF_FORMFILLINFO* param,
                                     FPDF_BYTESTRING uri) {
   PDFiumEngine* engine = static_cast<PDFiumEngine*>(param);
-  engine->client_->NavigateTo(std::string(uri), false);
+  engine->client_->NavigateTo(std::string(uri), CURRENT_TAB);
 }
 
 void PDFiumEngine::Form_DoGoToAction(FPDF_FORMFILLINFO* param,

pdf/preview_mode_client.cc

@@ -38,7 +38,7 @@ void PreviewModeClient::ScrollToPage(int page) {
 }
 
 void PreviewModeClient::NavigateTo(const std::string& url,
-                                   bool open_in_new_tab) {
+                                   WindowOpenDisposition disposition) {
   NOTREACHED();
 }
 

pdf/preview_mode_client.h

@@ -33,7 +33,8 @@ class PreviewModeClient : public PDFEngine::Client {
   void ScrollToX(int position) override;
   void ScrollToY(int position) override;
   void ScrollToPage(int page) override;
-  void NavigateTo(const std::string& url, bool open_in_new_tab) override;
+  void NavigateTo(const std::string& url,
+                  WindowOpenDisposition disposition) override;
   void UpdateCursor(PP_CursorType_Dev cursor) override;
   void UpdateTickMarks(const std::vector<pp::Rect>& tickmarks) override;
   void NotifyNumberOfFindResultsChanged(int total, bool final_result) override;