Allow Signin page to open other chrome:// URLs if login content in <webview>

chrome/browser/apps/guest_view/web_view_browsertest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/path_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/chrome_content_browser_client.h"
 #include "chrome/browser/prerender/prerender_link_manager.h"
 #include "chrome/browser/prerender/prerender_link_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
@@ skipping 154 matching lines @@
 // This class intercepts media access request from the embedder. The request
 // should be triggered only if the embedder API (from tests) allows the request
 // in Javascript.
 // We do not issue the actual media request; the fact that the request reached
 // embedder's WebContents is good enough for our tests. This is also to make
 // the test run successfully on trybots.
 class MockWebContentsDelegate : public content::WebContentsDelegate {
  public:
   MockWebContentsDelegate()
       : requested_(false),
-        checked_(false) {}
+        checked_(false),
+        open_url_from_tab_(false) {}
   ~MockWebContentsDelegate() override {}
 
   void RequestMediaAccessPermission(
       content::WebContents* web_contents,
       const content::MediaStreamRequest& request,
       const content::MediaResponseCallback& callback) override {
     requested_ = true;
     if (request_message_loop_runner_.get())
       request_message_loop_runner_->Quit();
   }
 
   bool CheckMediaAccessPermission(content::WebContents* web_contents,
                                   const GURL& security_origin,
                                   content::MediaStreamType type) override {
     checked_ = true;
     if (check_message_loop_runner_.get())
       check_message_loop_runner_->Quit();
     return true;
   }
 
+  content::WebContents* OpenURLFromTab(
+      content::WebContents* source,
+      const content::OpenURLParams& params) override {
+    open_url_from_tab_ = true;
+    if (open_url_message_loop_runner_.get())
+      open_url_message_loop_runner_->Quit();
+    return NULL;
+  }
+
   void WaitForRequestMediaPermission() {
     if (requested_)
       return;
     request_message_loop_runner_ = new content::MessageLoopRunner;
     request_message_loop_runner_->Run();
   }
 
   void WaitForCheckMediaPermission() {
     if (checked_)
       return;
     check_message_loop_runner_ = new content::MessageLoopRunner;
     check_message_loop_runner_->Run();
   }
 
+  void WaitForOpenURLFromTab() {
+    if (open_url_from_tab_)
+      return;
+    open_url_message_loop_runner_ = new content::MessageLoopRunner;
+    open_url_message_loop_runner_->Run();
+  }
+
  private:
   bool requested_;
   bool checked_;
+  bool open_url_from_tab_;

[Charlie Reis, 2015/02/19 22:56:28]

nit: Make past tense, e.g., did_open_url_from_tab_

[Fady Samuel, 2015/02/20 00:33:06]

Done.

   scoped_refptr<content::MessageLoopRunner> request_message_loop_runner_;
   scoped_refptr<content::MessageLoopRunner> check_message_loop_runner_;
+  scoped_refptr<content::MessageLoopRunner> open_url_message_loop_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(MockWebContentsDelegate);
 };
 
 // This class intercepts download request from the guest.
 class MockDownloadWebContentsDelegate : public content::WebContentsDelegate {
  public:
   explicit MockDownloadWebContentsDelegate(
       content::WebContentsDelegate* orig_delegate)
       : orig_delegate_(orig_delegate),
@@ skipping 1592 matching lines @@
   EXPECT_TRUE(
       content::ExecuteScript(
           embedder_web_contents,
           base::StringPrintf("startAllowTest('%s')",
                              test_name.c_str())));
   ASSERT_TRUE(done_listener.WaitUntilSatisfied());
 
   mock->WaitForRequestMediaPermission();
 }
 
+IN_PROC_BROWSER_TEST_F(WebViewTest, OpenURLFromTab_CurrentTab_Abort) {
+  LoadAppWithGuest("web_view/simple");
+
+  // Verify that OpenURLFromTab with a window disposition of CURRENT_TAB will
+  // navigate the current <webview>.
+  ExtensionTestMessageListener load_listener("WebViewTest.LOADSTOP", false);
+
+  // Navigating to a file URL is forbidden inside a <webview>.
+  content::OpenURLParams params(GURL("file://foo"),
+                                content::Referrer(),
+                                CURRENT_TAB,
+                                ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
+                                true /* is_renderer_initiated */);
+  GetGuestWebContents()->GetDelegate()->OpenURLFromTab(
+      GetGuestWebContents(), params);
+
+  ASSERT_TRUE(load_listener.WaitUntilSatisfied());
+
+  // Verify that the <webview> ends up at about:blank.
+  EXPECT_EQ(GURL(url::kAboutBlankURL),
+            GetGuestWebContents()->GetLastCommittedURL());
+}
+
+IN_PROC_BROWSER_TEST_F(WebViewTest, OpenURLFromTab_NewWindow_Abort) {
+  LoadAppWithGuest("web_view/simple");
+
+  // Verify that OpenURLFromTab with a window disposition of NEW_BACKGROUND_TAB
+  // will trigger the <webview>'s New Window API.
+  ExtensionTestMessageListener new_window_listener(
+      "WebViewTest.NEWWINDOW", false);
+
+  // Navigating to a file URL is forbidden inside a <webview>.
+  content::OpenURLParams params(GURL("file://foo"),
+                                content::Referrer(),
+                                NEW_BACKGROUND_TAB,
+                                ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
+                                true /* is_renderer_initiated */);
+  GetGuestWebContents()->GetDelegate()->OpenURLFromTab(
+      GetGuestWebContents(), params);
+
+  ASSERT_TRUE(new_window_listener.WaitUntilSatisfied());
+
+  content::WebContents* new_guest_web_contents=

[Charlie Reis, 2015/02/19 22:56:28]

nit: Comma before =

[Fady Samuel, 2015/02/20 00:33:06]

Done.

[Charlie Reis, 2015/02/20 00:37:59]

Ha!  Yes, I meant space, of course.

+      GetGuestViewManager()->GetLastGuestCreated();
+  // Verify that a new guest was created.

[Charlie Reis, 2015/02/19 22:56:28]

nit: Move comment above previous statement.

[Fady Samuel, 2015/02/20 00:33:06]

Done.

+  EXPECT_NE(GetGuestWebContents(), new_guest_web_contents);
+
+  // Verify that the new <webview> guest ends up at about:blank.
+  EXPECT_EQ(GURL(url::kAboutBlankURL),
+            new_guest_web_contents->GetLastCommittedURL());
+}
+
+IN_PROC_BROWSER_TEST_F(WebViewTest, ContextMenuLanguageSettings) {

[Charlie Reis, 2015/02/19 22:56:28]

nit: Let's add a comment about the test, saying that we expect this case to
succeed, unlike navigations initiated by the webview.

[Fady Samuel, 2015/02/20 00:33:06]

Done.

+  LoadAppWithGuest("web_view/context_menus/basic");
+
+  content::WebContents* guest_web_contents = GetGuestWebContents();
+  content::WebContents* embedder = GetEmbedderWebContents();
+  ASSERT_TRUE(embedder);
+  scoped_ptr<MockWebContentsDelegate> mock(new MockWebContentsDelegate());
+  embedder->SetDelegate(mock.get());
+
+  // Create and build our test context menu.
+  GURL page_url("http://www.google.com");
+  scoped_ptr<TestRenderViewContextMenu> menu(TestRenderViewContextMenu::Create(
+      guest_web_contents, page_url, GURL(), GURL()));
+  menu->ExecuteCommand(IDC_CONTENT_CONTEXT_LANGUAGE_SETTINGS, 0);
+
+  mock->WaitForOpenURLFromTab();

[Charlie Reis, 2015/02/19 22:56:28]

Can we verify something that shows the navigation succeeded?  Maybe add a
WebContentsAddedObserver and show that it gets to the language settings URL?

[Fady Samuel, 2015/02/20 00:33:06]

Done.

+}
+
 IN_PROC_BROWSER_TEST_F(WebViewTest, ContextMenusAPI_Basic) {
   LoadAppWithGuest("web_view/context_menus/basic");
 
   content::WebContents* guest_web_contents = GetGuestWebContents();
   content::WebContents* embedder = GetEmbedderWebContents();
   ASSERT_TRUE(embedder);
 
   // 1. Basic property test.
   ExecuteScriptWaitForTitle(embedder, "checkProperties()", "ITEM_CHECKED");
 
@@ skipping 105 matching lines @@
 #endif
 
 // Flaky on Windows. http://crbug.com/303966
 #if defined(OS_WIN)
 #define MAYBE_TearDownTest DISABLED_TearDownTest
 #else
 #define MAYBE_TearDownTest TearDownTest
 #endif
 IN_PROC_BROWSER_TEST_F(WebViewTest, MAYBE_TearDownTest) {
   const extensions::Extension* extension =
-      LoadAndLaunchPlatformApp("web_view/teardown", "guest-loaded");
+      LoadAndLaunchPlatformApp("web_view/simple", "WebViewTest.LAUNCHED");
   extensions::AppWindow* window = NULL;
   if (!GetAppWindowCount())
     window = CreateAppWindow(extension);
   else
     window = GetFirstAppWindow();
   CloseAppWindow(window);
 
   // Load the app again.
-  LoadAndLaunchPlatformApp("web_view/teardown", "guest-loaded");
+  LoadAndLaunchPlatformApp("web_view/simple", "WebViewTest.LAUNCHED");
 }
 
 // In following GeolocationAPIEmbedderHasNoAccess* tests, embedder (i.e. the
 // platform app) does not have geolocation permission for this test.
 // No matter what the API does, geolocation permission would be denied.
 // Note that the test name prefix must be "GeolocationAPI".
 IN_PROC_BROWSER_TEST_F(WebViewTest, GeolocationAPIEmbedderHasNoAccessAllow) {
   TestHelper("testDenyDenies",
              "web_view/geolocation/embedder_has_no_permission",
              NEEDS_TEST_SERVER);
@@ skipping 496 matching lines @@
 // http://crbug.com/403325
 #define MAYBE_WebViewInBackgroundPage \
     DISABLED_WebViewInBackgroundPage
 #else
 #define MAYBE_WebViewInBackgroundPage WebViewInBackgroundPage
 #endif
 IN_PROC_BROWSER_TEST_F(WebViewTest, MAYBE_WebViewInBackgroundPage) {
   ASSERT_TRUE(RunExtensionTest("platform_apps/web_view/background"))
       << message_;
 }