Don't allow MimeHandlerViews to navigate away from their handling extension

Currently, the PDF viewer extension tries to navigate itself using
|window.location.href| when navigating the tab via the chrome.tabs API
is not available. This fallback behaviour is incorrect as it would
navigate the guest contents instead of the tab.

We remove this fallback behaviour from the PDF viewer.

Bug: 1158381
Change-Id: I5295a60c0e3d05e7058be81b39c0716c9f6e8d65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2618602Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844080}

chrome/browser/pdf/pdf_extension_test.cc

@@ -1776,10 +1776,11 @@ class PDFExtensionLinkClickTest : public PDFExtensionTestWithParam {
   ~PDFExtensionLinkClickTest() override {}
 
  protected:
-  void LoadTestLinkPdfGetGuestContents() {
+  content::WebContents* LoadTestLinkPdfGetGuestContents() {
     GURL test_pdf_url(embedded_test_server()->GetURL("/pdf/test-link.pdf"));
     guest_contents_ = LoadPdfGetGuestContents(test_pdf_url);
-    ASSERT_TRUE(guest_contents_);
+    EXPECT_TRUE(guest_contents_);
+    return guest_contents_;
   }
 
   // The rectangle of the link in test-link.pdf is [72 706 164 719] in PDF user
@@ -1974,6 +1975,49 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionLinkClickTest, OpenPDFWithReplaceState) {
   EXPECT_EQ("http://www.example.com/", url.spec());
 }
 
+namespace {
+// Fails the test if a navigation is started in the given WebContents.
+class FailOnNavigation : public content::WebContentsObserver {
+ public:
+  explicit FailOnNavigation(content::WebContents* contents)
+      : content::WebContentsObserver(contents) {}
+
+  // content::WebContentsObserver:
+  void DidStartNavigation(
+      content::NavigationHandle* navigation_handle) override {
+    ADD_FAILURE() << "Unexpected navigation";
+  }
+};
+}  // namespace
+
+// If the PDF viewer can't navigate the tab using a tab id, make sure it doesn't
+// try to navigate the mime handler extension's frame.
+// Regression test for https://crbug.com/1158381
+IN_PROC_BROWSER_TEST_P(PDFExtensionLinkClickTest, LinkClickInPdfInNonTab) {
+  // For ease of testing, we'll still load the PDF in a tab, but we clobber the
+  // tab id in the viewer to make it think it's not in a tab.
+  WebContents* guest_contents = LoadTestLinkPdfGetGuestContents();
+  ASSERT_TRUE(guest_contents);
+  ASSERT_TRUE(
+      content::ExecuteScript(guest_contents,
+                             "window.viewer.browserApi.getStreamInfo().tabId = "
+                             "    chrome.tabs.TAB_ID_NONE;"));
+
+  FailOnNavigation fail_if_mimehandler_navigates(guest_contents);
+  content::SimulateMouseClickAt(
+      GetWebContentsForInputRouting(), blink::WebInputEvent::kNoModifiers,
+      blink::WebMouseEvent::Button::kLeft, GetLinkPosition());
+
+  // Since the guest contents is for a mime handling extension (in this case,
+  // the PDF viewer extension), it must not navigate away from the extension. If
+  // |fail_if_mimehandler_navigates| doesn't see a navigation, we consider the
+  // test to have passed.
+  base::RunLoop run_loop;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
+  run_loop.Run();
+}
+
 INSTANTIATE_TEST_SUITE_P(/* no prefix */,
                          PDFExtensionLinkClickTest,
                          testing::Bool());

chrome/browser/resources/pdf/BUILD.gn

@@ -245,6 +245,7 @@ js_library("metrics") {
 
 js_library("navigator") {
   deps = [
+    ":browser_api",
     ":open_pdf_params_parser",
     ":viewport",
   ]

chrome/browser/resources/pdf/browser_api.js

@@ -85,6 +85,27 @@ export class BrowserApi {
     return this.streamInfo_;
   }
 
+  /**
+   * Navigates the current tab.
+   * @param {string} url The URL to navigate the tab to.
+   */
+  navigateInCurrentTab(url) {
+    const tabId = this.getStreamInfo().tabId;
+    // We need to use the tabs API to navigate because
+    // |window.location.href| cannot be used. This PDF extension is not loaded
+    // in the top level frame (it's embedded using MimeHandlerView). Using
+    // |window.location| would navigate the wrong frame, so we can't
+    // use it as a fallback. If it turns out that we do need a way to navigate
+    // in non-tab cases, we would need to create another mechanism to
+    // communicate with MimeHandler code in the browser (e.g. via
+    // mimeHandlerPrivate), which could then navigate the correct frame.
+    // Furthermore, navigations to local resources would be blocked with
+    // |window.location|.
+    if (chrome.tabs && tabId !== chrome.tabs.TAB_ID_NONE) {
+      chrome.tabs.update(tabId, {url: url});
+    }
+  }
+
   /**
    * Sets the browser zoom.
    * @param {number} zoom The zoom factor to send to the browser.

chrome/browser/resources/pdf/navigator.js

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import {BrowserApi} from './browser_api.js';
 import {OpenPdfParamsParser} from './open_pdf_params_parser.js';
 import {Viewport} from './viewport.js';
 
@@ -33,24 +34,15 @@ export class NavigatorDelegate {
 // navigating.
 /** @implements {NavigatorDelegate} */
 export class NavigatorDelegateImpl {
-  /**
-   * @param {number} tabId The tab ID of the PDF viewer or -1 if the viewer is
-   *     not displayed in a tab.
-   */
-  constructor(tabId) {
-    /** @private {number} */
-    this.tabId_ = tabId;
+  /** @param {!BrowserApi} browserApi */
+  constructor(browserApi) {
+    /** @private {!BrowserApi} */
+    this.browserApi_ = browserApi;
   }
 
   /** @override */
   navigateInCurrentTab(url) {
-    // When the PDFviewer is inside a browser tab, prefer the tabs API because
-    // it can navigate from one file:// URL to another.
-    if (chrome.tabs && this.tabId_ !== -1) {
-      chrome.tabs.update(this.tabId_, {url: url});
-    } else {
-      window.location.href = url;
-    }
+    this.browserApi_.navigateInCurrentTab(url);
   }
 
   /** @override */

chrome/browser/resources/pdf/pdf_viewer.js

@@ -428,11 +428,10 @@ export class PDFViewerElement extends PDFViewerBaseElement {
         'keydown',
         e => this.handleKeyEvent_(/** @type {!KeyboardEvent} */ (e)));
 
-    const tabId = this.browserApi.getStreamInfo().tabId;
     this.navigator_ = new PdfNavigator(
         this.originalUrl, this.viewport,
         /** @type {!OpenPdfParamsParser} */ (this.paramsParser),
-        new NavigatorDelegateImpl(tabId));
+        new NavigatorDelegateImpl(browserApi));
 
     // Listen for save commands from the browser.
     if (chrome.mimeHandlerPrivate && chrome.mimeHandlerPrivate.onSave) {

extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc

@@ -217,6 +217,7 @@ void MimeHandlerViewGuest::CreateWebContents(
 }
 
 void MimeHandlerViewGuest::DidAttachToEmbedder() {
+  DCHECK(stream_->handler_url().SchemeIs(extensions::kExtensionScheme));
   web_contents()->GetController().LoadURL(
       stream_->handler_url(), content::Referrer(),
       ui::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string());
@@ -443,6 +444,18 @@ void MimeHandlerViewGuest::ReadyToCommitNavigation(
       stream_->TakeTransferrableURLLoader());
 }
 
+void MimeHandlerViewGuest::DidFinishNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (navigation_handle->IsInMainFrame()) {
+    // We should not navigate the guest away from the handling extension.
+    const url::Origin handler_origin =
+        url::Origin::Create(stream_->handler_url());
+    const url::Origin new_origin =
+        url::Origin::Create(navigation_handle->GetURL());
+    CHECK(new_origin.IsSameOriginWith(handler_origin));
+  }
+}
+
 void MimeHandlerViewGuest::FuseBeforeUnloadControl(
     mojo::PendingReceiver<mime_handler::BeforeUnloadControl> receiver) {
   if (!pending_before_unload_control_)

extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h

@@ -175,6 +175,7 @@ class MimeHandlerViewGuest
   void DocumentOnLoadCompletedInMainFrame() final;
   void ReadyToCommitNavigation(
       content::NavigationHandle* navigation_handle) final;
+  void DidFinishNavigation(content::NavigationHandle* navigation_handle) final;
 
   std::unique_ptr<MimeHandlerViewGuestDelegate> delegate_;
   std::unique_ptr<StreamContainer> stream_;