While trying to enable webview sign-in by default, I found a bunch of issues

that needed to fixed first.  This CL contains only those fixes.  Once this
lands I can commit the CL (795873004) that actually enables webview sign-in.  Fixes
here include:

- making Authenticator.js more of a drop in replacement for GaiaAuthHost.  Important for tests
- fix to some race conditions in Authenticator.js wrt loadstop
- support in inline_login to work with webview
- fixes to tests that were hardcoded to only test the old style sign-in

BUG=441821

Review URL: https://codereview.chromium.org/807503004

Cr-Commit-Position: refs/heads/master@{#310224}

12 files changed, 312 insertions, 309 deletions

diff --git a/chrome/browser/profiles/host_zoom_map_browsertest.cc b/chrome/browser/profiles/host_zoom_map_browsertest.cc
index 738d8f5..8baa41f 100644
--- a/chrome/browser/profiles/host_zoom_map_browsertest.cc
+++ b/chrome/browser/profiles/host_zoom_map_browsertest.cc
@@ -30,6 +30,7 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/signin/core/common/profile_management_switches.h"
 #include "components/ui/zoom/zoom_event_manager.h"
 #include "content/public/test/test_utils.h"
 #include "net/dns/mock_host_resolver.h"
@@ -257,9 +258,14 @@ IN_PROC_BROWSER_TEST_F(HostZoomMapBrowserTest,
   // Verify that our loaded page is using a HostZoomMap different from the
   // one for the default StoragePartition.
   HostZoomMap* host_zoom_map = HostZoomMap::GetForWebContents(web_contents);
-  HostZoomMap* default_profile_host_zoom_map =
-      HostZoomMap::GetDefaultForBrowserContext(browser()->profile());
-  EXPECT_NE(host_zoom_map, default_profile_host_zoom_map);
+
+  // For the webview based sign-in code, the sign in page uses the default host
+  // zoom map.
+  if (!switches::IsEnableWebviewBasedSignin()) {
+    HostZoomMap* default_profile_host_zoom_map =
+        HostZoomMap::GetDefaultForBrowserContext(browser()->profile());
+    EXPECT_NE(host_zoom_map, default_profile_host_zoom_map);
+  }
 
   double new_zoom_level =
       host_zoom_map->GetZoomLevelForHostAndScheme(test_scheme, test_host) + 0.5;
diff --git a/chrome/browser/resources/gaia_auth_host/authenticator.js b/chrome/browser/resources/gaia_auth_host/authenticator.js
index baf6d8e..133b023 100644
--- a/chrome/browser/resources/gaia_auth_host/authenticator.js
+++ b/chrome/browser/resources/gaia_auth_host/authenticator.js
@@ -12,6 +12,9 @@
 cr.define('cr.login', function() {
   'use strict';
 
+  // TODO(rogerta): should use gaia URL from GaiaUrls::gaia_url() instead
+  // of hardcoding the prod URL here.  As is, this does not work with staging
+  // environments.
   var IDP_ORIGIN = 'https://accounts.google.com/';
   var IDP_PATH = 'ServiceLogin?skipvpage=true&sarp=1&rm=hide';
   var CONTINUE_URL =
@@ -49,17 +52,12 @@ cr.define('cr.login', function() {
    * Initializes the authenticator component.
    * @param {webview|string} webview The webview element or its ID to host IdP
    *     web pages.
-   * @param {Authenticator.Listener=} opt_listener An optional listener for
-   *     authentication events.
    * @constructor
-   * @extends {cr.EventTarget}
    */
-  function Authenticator(webview, opt_listener) {
+  function Authenticator(webview) {
     this.webview_ = typeof webview == 'string' ? $(webview) : webview;
     assert(this.webview_);
 
-    this.listener_ = opt_listener || null;
-
     this.email_ = null;
     this.password_ = null;
     this.gaiaId_ = null,
@@ -73,6 +71,7 @@ cr.define('cr.login', function() {
     this.continueUrlWithoutParams_ = null;
     this.initialFrameUrl_ = null;
     this.reloadUrl_ = null;
+    this.trusted_ = true;
   }
 
   // TODO(guohui,xiyuan): no need to inherit EventTarget once we deprecate the
@@ -80,48 +79,6 @@ cr.define('cr.login', function() {
   Authenticator.prototype = Object.create(cr.EventTarget.prototype);
 
   /**
-   * An interface for receiving notifications upon authentication events.
-   * @interface
-   */
-  Authenticator.Listener = function() {};
-
-  /**
-   * Invoked when authentication UI is ready.
-   */
-  Authenticator.Listener.prototype.onReady = function(e) {};
-
-  /**
-   * Invoked when authentication is completed successfully with credential data.
-   * A credential data object looks like this:
-   * <pre>
-   * {@code
-   * {
-   *   email: 'xx@gmail.com',
-   *   password: 'xxxx',  // May be null or empty.
-   *   usingSAML: false,
-   *   chooseWhatToSync: false,
-   *   skipForNow: false,
-   *   sessionIndex: '0'
-   * }
-   * }
-   * </pre>
-   * @param {Object} credentials A credential data object.
-   */
-  Authenticator.Listener.prototype.onSuccess = function(credentials) {};
-
-  /**
-   * Invoked when the requested URL does not fit the container.
-   * @param {string} url Request URL.
-   */
-  Authenticator.Listener.prototype.onResize = function(url) {};
-
-  /**
-   * Invoked when a new window event is fired.
-   * @param {Event} e Event object.
-   */
-  Authenticator.Listener.prototype.onNewWindow = function(e) {};
-
-  /**
    * Loads the authenticator component with the given parameters.
    * @param {AuthMode} authMode Authorization mode.
    * @param {Object} data Parameters for the authorization flow.
@@ -141,6 +98,8 @@ cr.define('cr.login', function() {
     this.webview_.src = this.reloadUrl_;
     this.webview_.addEventListener(
         'newwindow', this.onNewWindow_.bind(this));
+    this.webview_.addEventListener(
+        'loadstop', this.onLoadStop_.bind(this));
     this.webview_.request.onCompleted.addListener(
         this.onRequestCompleted_.bind(this),
         {urls: ['*://*/*', this.continueUrlWithoutParams_ + '*'],
@@ -151,7 +110,7 @@ cr.define('cr.login', function() {
         {urls: [this.idpOrigin_ + '*'], types: ['main_frame']},
         ['responseHeaders']);
     window.addEventListener(
-        'message', this.onMessage_.bind(this), false);
+        'message', this.onMessageFromWebview_.bind(this), false);
   };
 
   /**
@@ -182,6 +141,7 @@ cr.define('cr.login', function() {
    */
   Authenticator.prototype.onRequestCompleted_ = function(details) {
     var currentUrl = details.url;
+
     if (currentUrl.lastIndexOf(this.continueUrlWithoutParams_, 0) == 0) {
       if (currentUrl.indexOf('ntp=1') >= 0) {
         this.skipForNow_ = true;
@@ -190,6 +150,10 @@ cr.define('cr.login', function() {
       return;
     }
 
+    if (currentUrl.indexOf('https') != 0) {
+      this.trusted_ = false;
+    }
+
     if (this.isConstrainedWindow_) {
       var isEmbeddedPage = false;
       if (this.idpOrigin_ && currentUrl.lastIndexOf(this.idpOrigin_) == 0) {
@@ -201,8 +165,8 @@ cr.define('cr.login', function() {
           }
         }
       }
-      if (!isEmbeddedPage && this.listener_) {
-        this.listener_.onResize(currentUrl);
+      if (!isEmbeddedPage) {
+        this.dispatchEvent(new CustomEvent('resize', {detail: currentUrl}));
         return;
       }
     }
@@ -210,13 +174,6 @@ cr.define('cr.login', function() {
     if (currentUrl.lastIndexOf(this.idpOrigin_) == 0) {
       this.webview_.contentWindow.postMessage({}, currentUrl);
     }
-
-    if (!this.loaded_) {
-      this.loaded_ = true;
-      if (this.listener_) {
-        this.listener_.onReady();
-      }
-    }
   };
 
   /**
@@ -254,17 +211,17 @@ cr.define('cr.login', function() {
   };
 
   /**
-   * Invoked when an HTML5 message is received.
+   * Invoked when an HTML5 message is received from the webview element.
    * @param {object} e Payload of the received HTML5 message.
    * @private
    */
-  Authenticator.prototype.onMessage_ = function(e) {
-    if (e.origin != this.idpOrigin_) {
+  Authenticator.prototype.onMessageFromWebview_ = function(e) {
+    // The event origin does not have a trailing slash.
+    if (e.origin != this.idpOrigin_.substring(0, this.idpOrigin_ - 1)) {
       return;
     }
 
     var msg = e.data;
-
     if (msg.method == 'attemptLogin') {
       this.email_ = msg.email;
       this.password_ = msg.password;
@@ -277,22 +234,21 @@ cr.define('cr.login', function() {
    * @private
    */
   Authenticator.prototype.onAuthCompleted_ = function() {
-    if (!this.listener_) {
-      return;
-    }
-
     if (!this.email_ && !this.skipForNow_) {
       this.webview_.src = this.initialFrameUrl_;
       return;
     }
 
-    this.listener_.onSuccess({email: this.email_,
-                              gaiaId: this.gaiaId_,
-                              password: this.password_,
-                              usingSAML: this.authFlow_ == AuthFlow.SAML,
-                              chooseWhatToSync: this.chooseWhatToSync_,
-                              skipForNow: this.skipForNow_,
-                              sessionIndex: this.sessionIndex_ || ''});
+    this.dispatchEvent(
+        new CustomEvent('authCompleted',
+                        {detail: {email: this.email_,
+                                  gaiaId: this.gaiaId_,
+                                  password: this.password_,
+                                  usingSAML: this.authFlow_ == AuthFlow.SAML,
+                                  chooseWhatToSync: this.chooseWhatToSync_,
+                                  skipForNow: this.skipForNow_,
+                                  sessionIndex: this.sessionIndex_ || '',
+                                  trusted: this.trusted_}}));
   };
 
   /**
@@ -300,11 +256,18 @@ cr.define('cr.login', function() {
    * @private
    */
   Authenticator.prototype.onNewWindow_ = function(e) {
-    if (!this.listener_) {
-      return;
-    }
+    this.dispatchEvent(new CustomEvent('newWindow', {detail: e}));
+  };
 
-    this.listener_.onNewWindow(e);
+  /**
+   * Invoked when the webview finishes loading a page.
+   * @private
+   */
+  Authenticator.prototype.onLoadStop_ = function(e) {
+    if (!this.loaded_) {
+      this.loaded_ = true;
+      this.dispatchEvent(new Event('ready'));
+    }
   };
 
   Authenticator.AuthFlow = AuthFlow;
diff --git a/chrome/browser/resources/inline_login/inline_login.js b/chrome/browser/resources/inline_login/inline_login.js
index ec86c44..22b1bbe 100644
--- a/chrome/browser/resources/inline_login/inline_login.js
+++ b/chrome/browser/resources/inline_login/inline_login.js
@@ -20,47 +20,25 @@ cr.define('inline.login', function() {
    */
   var authReadyFired;
 
-  /**
-   * A listener class for authentication events from GaiaAuthHost.
-   * @constructor
-   * @implements {cr.login.GaiaAuthHost.Listener}
-   */
-  function GaiaAuthHostListener() {}
-
-  /** @override */
-  GaiaAuthHostListener.prototype.onSuccess = function(credentials) {
-    onAuthCompleted(credentials);
-  };
-
-  /** @override */
-  GaiaAuthHostListener.prototype.onReady = function(e) {
-    onAuthReady();
-  };
+  function onResize(e) {
+    chrome.send('switchToFullTab', [e.detail]);
+  }
 
-  /** @override */
-  GaiaAuthHostListener.prototype.onResize = function(url) {
-    chrome.send('switchToFullTab', [url]);
-  };
+  function onAuthReady(e) {
+    $('contents').classList.toggle('loading', false);
+    authReadyFired = true;
+  }
 
-  /** @override */
-  GaiaAuthHostListener.prototype.onNewWindow = function(e) {
-    window.open(e.targetUrl, '_blank');
+  function onNewWindow(e) {
+    window.open(e.detail.targetUrl, '_blank');
     e.window.discard();
-  };
+  }
 
-  /**
-   * Handler of auth host 'ready' event.
-   */
-  function onAuthReady() {
-    $('contents').classList.toggle('loading', false);
-    authReadyFired = true;
+  function onAuthCompleted(e) {
+    completeLogin(e.detail);
   }
 
-  /**
-   * Handler of auth host 'completed' event.
-   * @param {!Object} credentials Credentials of the completed authentication.
-   */
-  function onAuthCompleted(credentials) {
+  function completeLogin(credentials) {
     chrome.send('completeLogin', [credentials]);
     $('contents').classList.toggle('loading', true);
   }
@@ -69,10 +47,11 @@ cr.define('inline.login', function() {
    * Initialize the UI.
    */
   function initialize() {
-    authExtHost = new cr.login.GaiaAuthHost(
-        'signin-frame', new GaiaAuthHostListener());
+    authExtHost = new cr.login.GaiaAuthHost('signin-frame');
     authExtHost.addEventListener('ready', onAuthReady);
-
+    authExtHost.addEventListener('newWindow', onNewWindow);
+    authExtHost.addEventListener('resize', onResize);
+    authExtHost.addEventListener('authCompleted', onAuthCompleted);
     chrome.send('initialize');
   }
 
@@ -81,7 +60,9 @@ cr.define('inline.login', function() {
    * @param {Object} data Parameters for auth extension.
    */
   function loadAuthExtension(data) {
-    authExtHost.load(data.authMode, data, onAuthCompleted);
+    // TODO(rogerta): in when using webview, the |completeLogin| argument
+    // is ignored.  See addEventListener() call above.
+    authExtHost.load(data.authMode, data, completeLogin);
     $('contents').classList.toggle('loading',
         data.authMode != cr.login.GaiaAuthHost.AuthMode.DESKTOP ||
         data.constrained == '1');
diff --git a/chrome/browser/signin/signin_browsertest.cc b/chrome/browser/signin/signin_browsertest.cc
index c06a083..45c5884 100644
--- a/chrome/browser/signin/signin_browsertest.cc
+++ b/chrome/browser/signin/signin_browsertest.cc
@@ -18,6 +18,7 @@
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/signin/core/common/profile_management_switches.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
@@ -107,6 +108,10 @@ const bool kOneClickSigninEnabled = false;
 #define MAYBE_ProcessIsolation ProcessIsolation
 #endif
 IN_PROC_BROWSER_TEST_F(SigninBrowserTest, MAYBE_ProcessIsolation) {
+  // This test is not needed for the webview based sign-in code.
+  if (switches::IsEnableWebviewBasedSignin())
+    return;
+
   SigninClient* signin =
       ChromeSigninClientFactory::GetForProfile(browser()->profile());
   EXPECT_FALSE(signin->HasSigninProcess());
@@ -156,6 +161,10 @@ IN_PROC_BROWSER_TEST_F(SigninBrowserTest, MAYBE_ProcessIsolation) {
 #endif
 
 IN_PROC_BROWSER_TEST_F(SigninBrowserTest, MAYBE_NotTrustedAfterRedirect) {
+  // This test is not needed for the webview based sign-in code.
+  if (switches::IsEnableWebviewBasedSignin())
+    return;
+
   SigninClient* signin =
       ChromeSigninClientFactory::GetForProfile(browser()->profile());
   EXPECT_FALSE(signin->HasSigninProcess());
@@ -206,6 +215,11 @@ class BackOnNTPCommitObserver : public content::WebContentsObserver {
 // and initiates a back navigation between the point of Commit and
 // DidStopLoading of the NTP.
 IN_PROC_BROWSER_TEST_F(SigninBrowserTest, SigninSkipForNowAndGoBack) {
+  // This test is not needed for the webview based sign-in code.
+  // OneClickSigninHelper is not used.
+  if (switches::IsEnableWebviewBasedSignin())
+    return;
+
   GURL ntp_url(chrome::kChromeUINewTabURL);
   GURL start_url = signin::GetPromoURL(
       signin_metrics::SOURCE_START_PAGE, false);
diff --git a/chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_browsertest.cc b/chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_browsertest.cc
new file mode 100644
index 0000000..536c1ea
--- /dev/null
+++ b/chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_browsertest.cc
@@ -0,0 +1,124 @@
+// 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 "chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate.h"
+
+#include "base/basictypes.h"
+#include "base/command_line.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/extensions/test_extension_service.h"
+#include "chrome/browser/extensions/test_extension_system.h"
+#include "chrome/browser/ui/bookmarks/bookmark_bubble_delegate.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/singleton_tabs.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/test/test_utils.h"
+#include "ui/events/event_constants.h"
+#include "ui/gfx/range/range.h"
+
+class BookmarkBubbleSignInDelegateTest : public InProcessBrowserTest {
+ public:
+  BookmarkBubbleSignInDelegateTest() {}
+
+  Profile* profile() { return browser()->profile(); }
+
+  void ReplaceBlank(Browser* browser);
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BookmarkBubbleSignInDelegateTest);
+};
+
+// The default browser created for tests start with one tab open on
+// about:blank.  The sign-in page is a singleton that will
+// replace this tab.  This function replaces about:blank with another URL
+// so that the sign in page goes into a new tab.
+void BookmarkBubbleSignInDelegateTest::ReplaceBlank(Browser* browser) {
+  chrome::NavigateParams params(
+      chrome::GetSingletonTabNavigateParams(browser, GURL("chrome:version")));
+  params.path_behavior = chrome::NavigateParams::IGNORE_AND_NAVIGATE;
+  chrome::ShowSingletonTabOverwritingNTP(browser, params);
+}
+
+IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, OnSignInLinkClicked) {
+  ReplaceBlank(browser());
+  int starting_tab_count = browser()->tab_strip_model()->count();
+
+  scoped_ptr<BookmarkBubbleDelegate> delegate;
+  delegate.reset(new BookmarkBubbleSignInDelegate(browser()));
+
+  delegate->OnSignInLinkClicked();
+
+  // A new tab should have been opened and the browser should be visible.
+  EXPECT_EQ(starting_tab_count + 1, browser()->tab_strip_model()->count());
+}
+
+IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest,
+                       OnSignInLinkClickedReusesBlank) {
+  int starting_tab_count = browser()->tab_strip_model()->count();
+
+  scoped_ptr<BookmarkBubbleDelegate> delegate;
+  delegate.reset(new BookmarkBubbleSignInDelegate(browser()));
+
+  delegate->OnSignInLinkClicked();
+
+  // A new tab should have been opened and the browser should be visible.
+  EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
+}
+
+IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest,
+                       OnSignInLinkClickedIncognito) {
+  ReplaceBlank(browser());
+  Browser* incognito_browser = CreateIncognitoBrowser();
+
+  int starting_tab_count_normal = browser()->tab_strip_model()->count();
+  int starting_tab_count_incognito =
+      incognito_browser->tab_strip_model()->count();
+
+  scoped_ptr<BookmarkBubbleDelegate> delegate;
+  delegate.reset(new BookmarkBubbleSignInDelegate(incognito_browser));
+
+  delegate->OnSignInLinkClicked();
+
+  // A new tab should have been opened in the normal browser, which should be
+  // visible.
+  int tab_count_normal = browser()->tab_strip_model()->count();
+  EXPECT_EQ(starting_tab_count_normal + 1, tab_count_normal);
+
+  // No effect is expected on the incognito browser.
+  int tab_count_incognito = incognito_browser->tab_strip_model()->count();
+  EXPECT_EQ(starting_tab_count_incognito, tab_count_incognito);
+}
+
+// Verifies that the sign in page can be loaded in a different browser
+// if the provided browser is invalidated.
+IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, BrowserRemoved) {
+  // Create an extra browser.
+  Browser* extra_browser = CreateBrowser(profile());
+  ReplaceBlank(extra_browser);
+
+  int starting_tab_count = extra_browser->tab_strip_model()->count();
+
+  scoped_ptr<BookmarkBubbleDelegate> delegate;
+  delegate.reset(new BookmarkBubbleSignInDelegate(browser()));
+
+  BrowserList::SetLastActive(extra_browser);
+
+  // Close all tabs in the original browser.  Run all pending messages
+  // to make sure the browser window closes before continuing.
+  browser()->tab_strip_model()->CloseAllTabs();
+  content::RunAllPendingInMessageLoop();
+
+  delegate->OnSignInLinkClicked();
+
+  // A new tab should have been opened in the extra browser, which should be
+  // visible.
+  int tab_count = extra_browser->tab_strip_model()->count();
+  EXPECT_EQ(starting_tab_count + 1, tab_count);
+}
diff --git a/chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_unittest.cc b/chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_unittest.cc
deleted file mode 100644
index 233bf8a..0000000
--- a/chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_unittest.cc
+++ /dev/null
@@ -1,149 +0,0 @@
-// 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 "chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate.h"
-
-#include "base/basictypes.h"
-#include "base/command_line.h"
-#include "base/memory/scoped_ptr.h"
-#include "chrome/browser/extensions/test_extension_service.h"
-#include "chrome/browser/extensions/test_extension_system.h"
-#include "chrome/browser/ui/bookmarks/bookmark_bubble_delegate.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_list.h"
-#include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/common/chrome_switches.h"
-#include "chrome/test/base/browser_with_test_window_test.h"
-#include "chrome/test/base/testing_profile.h"
-#include "ui/events/event_constants.h"
-#include "ui/gfx/range/range.h"
-
-class BookmarkBubbleSignInDelegateTest : public BrowserWithTestWindowTest {
- public:
-  BookmarkBubbleSignInDelegateTest() {}
-
-  void SetUp() override;
-
- protected:
-  class Window : public TestBrowserWindow {
-   public:
-    Window() : show_count_(0) {}
-
-    int show_count() { return show_count_; }
-
-   private:
-    // TestBrowserWindow:
-    void Show() override { ++show_count_; }
-
-    // Number of times that the Show() method has been called.
-    int show_count_;
-
-    DISALLOW_COPY_AND_ASSIGN(Window);
-  };
-
-  BrowserWindow* CreateBrowserWindow() override { return new Window(); }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(BookmarkBubbleSignInDelegateTest);
-};
-
-void BookmarkBubbleSignInDelegateTest::SetUp() {
-  BrowserWithTestWindowTest::SetUp();
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-  // Force web-based signin, otherwise tests will crash because inline signin
-  // involves IO thread operation.
-  // TODO(guohui): fix the test for inline signin.
-  command_line->AppendSwitch(switches::kEnableWebBasedSignin);
-  // Adds TestExtensionSystem, since signin uses the gaia auth extension.
-  static_cast<extensions::TestExtensionSystem*>(
-      extensions::ExtensionSystem::Get(profile()))->CreateExtensionService(
-          command_line, base::FilePath(), false);
-}
-
-TEST_F(BookmarkBubbleSignInDelegateTest, OnSignInLinkClicked) {
-  int starting_tab_count = browser()->tab_strip_model()->count();
-
-  scoped_ptr<BookmarkBubbleDelegate> delegate;
-  delegate.reset(new BookmarkBubbleSignInDelegate(browser()));
-
-  delegate->OnSignInLinkClicked();
-
-  // A new tab should have been opened and the browser should be visible.
-  int tab_count = browser()->tab_strip_model()->count();
-  EXPECT_EQ(starting_tab_count + 1, tab_count);
-  EXPECT_LE(1,
-            static_cast<BookmarkBubbleSignInDelegateTest::Window*>(
-                browser()->window())->show_count());
-}
-
-TEST_F(BookmarkBubbleSignInDelegateTest, OnSignInLinkClickedIncognito) {
-  scoped_ptr<BrowserWindow> incognito_window;
-  incognito_window.reset(CreateBrowserWindow());
-  Browser::CreateParams params(browser()->profile()->GetOffTheRecordProfile(),
-                               browser()->host_desktop_type());
-  params.window = incognito_window.get();
-  scoped_ptr<Browser> incognito_browser;
-  incognito_browser.reset(new Browser(params));
-
-  int starting_tab_count_normal = browser()->tab_strip_model()->count();
-  int starting_tab_count_incognito =
-      incognito_browser.get()->tab_strip_model()->count();
-
-  scoped_ptr<BookmarkBubbleDelegate> delegate;
-  delegate.reset(new BookmarkBubbleSignInDelegate(incognito_browser.get()));
-
-  delegate->OnSignInLinkClicked();
-
-  // A new tab should have been opened in the normal browser, which should be
-  // visible.
-  int tab_count_normal = browser()->tab_strip_model()->count();
-  EXPECT_EQ(starting_tab_count_normal + 1, tab_count_normal);
-  EXPECT_LE(1,
-            static_cast<BookmarkBubbleSignInDelegateTest::Window*>(
-                browser()->window())->show_count());
-
-  // No effect is expected on the incognito browser.
-  int tab_count_incognito = incognito_browser->tab_strip_model()->count();
-  EXPECT_EQ(starting_tab_count_incognito, tab_count_incognito);
-  EXPECT_EQ(0,
-            static_cast<BookmarkBubbleSignInDelegateTest::Window*>(
-                incognito_window.get())->show_count());
-}
-
-// Verifies that the sign in page can be loaded in a different browser
-// if the provided browser is invalidated.
-TEST_F(BookmarkBubbleSignInDelegateTest, BrowserRemoved) {
-  // Create an extra browser.
-  scoped_ptr<BrowserWindow> extra_window;
-  extra_window.reset(CreateBrowserWindow());
-
-  Browser::CreateParams params(browser()->profile(),
-                               browser()->host_desktop_type());
-  params.window = extra_window.get();
-  scoped_ptr<Browser> extra_browser;
-  extra_browser.reset(new Browser(params));
-
-  int starting_tab_count = extra_browser->tab_strip_model()->count();
-
-  scoped_ptr<BookmarkBubbleDelegate> delegate;
-  delegate.reset(new BookmarkBubbleSignInDelegate(browser()));
-
-  BrowserList::SetLastActive(extra_browser.get());
-
-  browser()->tab_strip_model()->CloseAllTabs();
-  set_browser(NULL);
-
-  delegate->OnSignInLinkClicked();
-
-  // A new tab should have been opened in the extra browser, which should be
-  // visible.
-  int tab_count = extra_browser->tab_strip_model()->count();
-  EXPECT_EQ(starting_tab_count + 1, tab_count);
-  EXPECT_LE(1,
-            static_cast<BookmarkBubbleSignInDelegateTest::Window*>(
-                extra_window.get())->show_count());
-
-  // Required to avoid a crash when the browser is deleted.
-  extra_browser->tab_strip_model()->CloseAllTabs();
-}
diff --git a/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc b/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc
index f9452bd..89f80a9 100644
--- a/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc
+++ b/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc
@@ -240,6 +240,7 @@ InlineLoginHandlerImpl::InlineLoginHandlerImpl()
 
 InlineLoginHandlerImpl::~InlineLoginHandlerImpl() {}
 
+// This method is not called with webview sign in enabled.
 void InlineLoginHandlerImpl::DidCommitProvisionalLoadForFrame(
     content::RenderFrameHost* render_frame_host,
     const GURL& url,
@@ -294,6 +295,11 @@ void InlineLoginHandlerImpl::CompleteLogin(const base::ListValue* args) {
     return;
   }
 
+  // This value exists only for webview sign in.
+  bool trusted = false;
+  if (dict->GetBoolean("trusted", &trusted))
+    confirm_untrusted_signin_ = !trusted;
+
   base::string16 email_string16;
   dict->GetString("email", &email_string16);
   DCHECK(!email_string16.empty());
diff --git a/chrome/browser/ui/webui/signin/inline_login_ui.cc b/chrome/browser/ui/webui/signin/inline_login_ui.cc
index 4e32062..fe2980a 100644
--- a/chrome/browser/ui/webui/signin/inline_login_ui.cc
+++ b/chrome/browser/ui/webui/signin/inline_login_ui.cc
@@ -14,6 +14,7 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_ui.h"
 #include "content/public/browser/web_ui_data_source.h"
+#include "extensions/browser/guest_view/guest_view_manager.h"
 #include "grit/browser_resources.h"
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/ui/webui/chromeos/login/inline_login_handler_chromeos.h"
@@ -54,6 +55,12 @@ void AddToSetIfIsAuthIframe(std::set<content::RenderFrameHost*>* frame_set,
   }
 }
 
+bool AddToSetIfSigninWebview(std::set<content::RenderFrameHost*>* frame_set,
+                             content::WebContents* web_contents) {
+  frame_set->insert(web_contents->GetMainFrame());
+  return false;
+}
+
 } // empty namespace
 
 InlineLoginUI::InlineLoginUI(content::WebUI* web_ui)
@@ -88,9 +95,17 @@ content::RenderFrameHost* InlineLoginUI::GetAuthIframe(
     const GURL& parent_origin,
     const std::string& parent_frame_name) {
   std::set<content::RenderFrameHost*> frame_set;
-  web_contents->ForEachFrame(
-      base::Bind(&AddToSetIfIsAuthIframe, &frame_set,
-                 parent_origin, parent_frame_name));
+  if (switches::IsEnableWebviewBasedSignin()) {
+    extensions::GuestViewManager* manager =
+        extensions::GuestViewManager::FromBrowserContext(
+            web_contents->GetBrowserContext());
+    manager->ForEachGuest(web_contents,
+                          base::Bind(&AddToSetIfSigninWebview, &frame_set));
+  } else {
+    web_contents->ForEachFrame(
+        base::Bind(&AddToSetIfIsAuthIframe, &frame_set,
+                   parent_origin, parent_frame_name));
+  }
   DCHECK_GE(1U, frame_set.size());
   if (!frame_set.empty())
     return *frame_set.begin();
diff --git a/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc b/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc
index 8378ecf..0f8d24e 100644
--- a/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc
+++ b/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc
@@ -17,6 +17,7 @@
 #include "chrome/test/base/test_chrome_web_ui_controller_factory.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/signin/core/common/profile_management_switches.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/session_storage_namespace.h"
@@ -26,6 +27,7 @@
 #include "content/public/common/url_constants.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "extensions/browser/guest_view/guest_view_manager.h"
 #include "google_apis/gaia/fake_gaia.h"
 #include "google_apis/gaia/gaia_switches.h"
 #include "net/base/url_util.h"
@@ -45,11 +47,15 @@ using login_ui_test_utils::WaitUntilUIReady;
 namespace {
 
 struct ContentInfo {
-  ContentInfo(int pid, content::StoragePartition* storage_partition) {
+  ContentInfo(content::WebContents* contents,
+              int pid,
+              content::StoragePartition* storage_partition) {
+    this->contents = contents;
     this->pid = pid;
     this->storage_partition = storage_partition;
   }
 
+  content::WebContents* contents;
   int pid;
   content::StoragePartition* storage_partition;
 };
@@ -64,7 +70,8 @@ ContentInfo NavigateAndGetInfo(
   content::WebContents* contents =
       browser->tab_strip_model()->GetActiveWebContents();
   content::RenderProcessHost* process = contents->GetRenderProcessHost();
-  return ContentInfo(process->GetID(), process->GetStoragePartition());
+  return ContentInfo(contents, process->GetID(),
+                     process->GetStoragePartition());
 }
 
 // Returns a new WebUI object for the WebContents from |arg0|.
@@ -88,6 +95,12 @@ class MockLoginUIObserver : public LoginUIService::Observer {
 
 const char kFooWebUIURL[] = "chrome://foo/";
 
+bool AddToSet(std::set<content::WebContents*>* set,
+              content::WebContents* web_contents) {
+  set->insert(web_contents);
+  return false;
+}
+
 }  // namespace
 
 class InlineLoginUIBrowserTest : public InProcessBrowserTest {
@@ -102,26 +115,48 @@ class InlineLoginUIBrowserTest : public InProcessBrowserTest {
 #define MAYBE_DifferentStorageId DifferentStorageId
 #endif
 IN_PROC_BROWSER_TEST_F(InlineLoginUIBrowserTest, MAYBE_DifferentStorageId) {
-  GURL test_url = ui_test_utils::GetTestUrl(
-      base::FilePath(base::FilePath::kCurrentDirectory),
-      base::FilePath(FILE_PATH_LITERAL("title1.html")));
-
-  ContentInfo info1 =
-      NavigateAndGetInfo(browser(), test_url, CURRENT_TAB);
-  ContentInfo info2 = NavigateAndGetInfo(
-      browser(),
-      signin::GetPromoURL(signin_metrics::SOURCE_START_PAGE, false),
-      CURRENT_TAB);
-  NavigateAndGetInfo(browser(), test_url, CURRENT_TAB);
-  ContentInfo info3 = NavigateAndGetInfo(
-      browser(),
-      signin::GetPromoURL(signin_metrics::SOURCE_START_PAGE, false),
-      NEW_FOREGROUND_TAB);
-
-  // The info for signin should be the same.
-  ASSERT_EQ(info2.storage_partition, info3.storage_partition);
-  // The info for test_url and signin should be different.
-  ASSERT_NE(info1.storage_partition, info2.storage_partition);
+  if (switches::IsEnableWebviewBasedSignin()) {
+    ContentInfo info = NavigateAndGetInfo(
+        browser(),
+        signin::GetPromoURL(signin_metrics::SOURCE_START_PAGE, false),
+        CURRENT_TAB);
+    WaitUntilUIReady(browser());
+
+    // Make sure storage partition of embedded webview is different from
+    // parent.
+    std::set<content::WebContents*> set;
+    extensions::GuestViewManager* manager =
+        extensions::GuestViewManager::FromBrowserContext(
+            info.contents->GetBrowserContext());
+    manager->ForEachGuest(info.contents, base::Bind(&AddToSet, &set));
+    ASSERT_EQ(1u, set.size());
+    content::WebContents* webview_contents = *set.begin();
+    content::RenderProcessHost* process =
+        webview_contents->GetRenderProcessHost();
+    ASSERT_NE(info.pid, process->GetID());
+    ASSERT_NE(info.storage_partition, process->GetStoragePartition());
+  } else {
+    GURL test_url = ui_test_utils::GetTestUrl(
+        base::FilePath(base::FilePath::kCurrentDirectory),
+        base::FilePath(FILE_PATH_LITERAL("title1.html")));
+
+    ContentInfo info1 =
+        NavigateAndGetInfo(browser(), test_url, CURRENT_TAB);
+    ContentInfo info2 = NavigateAndGetInfo(
+        browser(),
+        signin::GetPromoURL(signin_metrics::SOURCE_START_PAGE, false),
+        CURRENT_TAB);
+    NavigateAndGetInfo(browser(), test_url, CURRENT_TAB);
+    ContentInfo info3 = NavigateAndGetInfo(
+        browser(),
+        signin::GetPromoURL(signin_metrics::SOURCE_START_PAGE, false),
+        NEW_FOREGROUND_TAB);
+
+    // The info for signin should be the same.
+    ASSERT_EQ(info2.storage_partition, info3.storage_partition);
+    // The info for test_url and signin should be different.
+    ASSERT_NE(info1.storage_partition, info2.storage_partition);
+  }
 }
 
 IN_PROC_BROWSER_TEST_F(InlineLoginUIBrowserTest, OneProcessLimit) {
@@ -277,7 +312,7 @@ IN_PROC_BROWSER_TEST_F(InlineLoginUISafeIframeBrowserTest,
       base::Bind(&FakeGaia::HandleRequest,
                  base::Unretained(&fake_gaia)));
   fake_gaia.SetFakeMergeSessionParams(
-      "email", "fake-sid-cookie", "fake-lsid-cookie");
+      "email@gmail.com", "fake-sid-cookie", "fake-lsid-cookie");
 
   // Navigates to the Chrome signin page which loads the fake gaia auth page.
   // Since the fake gaia auth page is served over HTTP, thus expects to see an
@@ -293,7 +328,7 @@ IN_PROC_BROWSER_TEST_F(InlineLoginUISafeIframeBrowserTest,
   EXPECT_CALL(observer, OnUntrustedLoginUIShown())
       .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
 
-  ExecuteJsToSigninInSigninFrame(browser(), "email", "password");
+  ExecuteJsToSigninInSigninFrame(browser(), "email@gmail.com", "password");
   run_loop.Run();
   base::MessageLoop::current()->RunUntilIdle();
 }
diff --git a/chrome/browser/ui/zoom/zoom_controller_browsertest.cc b/chrome/browser/ui/zoom/zoom_controller_browsertest.cc
index 8639474be..d11fd1b 100644
--- a/chrome/browser/ui/zoom/zoom_controller_browsertest.cc
+++ b/chrome/browser/ui/zoom/zoom_controller_browsertest.cc
@@ -9,10 +9,12 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/webui/signin/login_ui_test_utils.h"
 #include "chrome/browser/ui/zoom/chrome_zoom_level_prefs.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/signin/core/common/profile_management_switches.h"
 #include "content/public/browser/host_zoom_map.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_process_host.h"
@@ -161,6 +163,7 @@ IN_PROC_BROWSER_TEST_F(ZoomControllerBrowserTest, Observe) {
   zoom_change_watcher.Wait();
 }
 
+#if !defined(OS_CHROMEOS)
 // Regression test: crbug.com/438979.
 IN_PROC_BROWSER_TEST_F(ZoomControllerBrowserTest,
                        SettingsZoomAfterSigninWorks) {
@@ -171,12 +174,10 @@ IN_PROC_BROWSER_TEST_F(ZoomControllerBrowserTest,
   // backs the signin page. When we subsequently navigate away from the
   // signin page, the HostZoomMap changes, and we need to test that the
   // ZoomController correctly detects this.
-  // TODO(wjmaclean): It would be nice if detecting when the signin page is
-  // fully loaded without needing a hard-coded value.
-  const int kLoadStopsBeforeSigninPageIsFullyLoaded = 3;
-  ui_test_utils::NavigateToURLWithDispositionBlockUntilNavigationsComplete(
-      browser(), signin_url, kLoadStopsBeforeSigninPageIsFullyLoaded,
-      NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), signin_url, NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  login_ui_test_utils::WaitUntilUIReady(browser());
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   EXPECT_NE(
@@ -205,12 +206,16 @@ IN_PROC_BROWSER_TEST_F(ZoomControllerBrowserTest,
   EXPECT_EQ(settings_url, web_contents->GetLastCommittedURL());
   EXPECT_EQ(zoom_controller, ZoomController::FromWebContents(web_contents));
 
-  // We expect the navigation from the chrome sign in page to the settings
-  // page to invoke a storage partition switch, and thus a different HostZoomMap
-  // for the web_contents.
-  content::HostZoomMap* host_zoom_map_settings =
-      content::HostZoomMap::GetForWebContents(web_contents);
-  EXPECT_NE(host_zoom_map_signin, host_zoom_map_settings);
+  // For the webview based sign-in code, the sign in page uses the default host
+  // zoom map.
+  if (!switches::IsEnableWebviewBasedSignin()) {
+    // We expect the navigation from the chrome sign in page to the settings
+    // page to invoke a storage partition switch, and thus a different
+    // HostZoomMap for the web_contents.
+    content::HostZoomMap* host_zoom_map_settings =
+        content::HostZoomMap::GetForWebContents(web_contents);
+    EXPECT_NE(host_zoom_map_signin, host_zoom_map_settings);
+  }
 
   // If we zoom the new page, it should still generate a ZoomController event.
   double old_zoom_level = zoom_controller->GetZoomLevel();
@@ -226,3 +231,4 @@ IN_PROC_BROWSER_TEST_F(ZoomControllerBrowserTest,
   zoom_controller->SetZoomLevel(new_zoom_level);
   zoom_change_watcher.Wait();
 }
+#endif  // !defined(OS_CHROMEOS)
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 106ee75..02476d3 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -433,6 +433,7 @@
       'browser/ui/autofill/test_generated_credit_card_bubble_controller.h',
       'browser/ui/blocked_content/popup_blocker_browsertest.cc',
       'browser/ui/bookmarks/bookmark_browsertest.cc',
+      'browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_browsertest.cc',
       'browser/ui/browser_browsertest.cc',
       'browser/ui/browser_close_browsertest.cc',
       'browser/ui/browser_command_controller_browsertest.cc',
@@ -2124,6 +2125,8 @@
             'browser/profiles/profile_list_desktop_browsertest.cc',
             'browser/service_process/service_process_control_browsertest.cc',
             'browser/signin/signin_browsertest.cc',
+            # bookmark sign in promo not used on chromeos
+            'browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_browsertest.cc',
             # chromeos does not use cross-platform panels
             'browser/ui/panels/panel_extension_browsertest.cc',
             # chromeos does not use the desktop user manager
@@ -2292,6 +2295,7 @@
           'sources!': [
             'browser/policy/cloud/component_cloud_policy_browsertest.cc',
             'browser/prefs/pref_hash_browsertest.cc',
+            'browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_browsertest.cc',
           ],
         }],
         ['os_posix == 1 and OS != "mac" and OS != "android"', {
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index 6b9c2b3..e7e1dc3 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -926,7 +926,6 @@
       'browser/ui/autofill/test_popup_controller_common.cc',
       'browser/ui/autofill/test_popup_controller_common.h',
       'browser/ui/bookmarks/bookmark_context_menu_controller_unittest.cc',
-      'browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_unittest.cc',
       'browser/ui/bookmarks/bookmark_editor_unittest.cc',
       'browser/ui/bookmarks/bookmark_ui_utils_unittest.cc',
       'browser/ui/bookmarks/bookmark_unittest.cc',
@@ -2618,7 +2617,6 @@
             'browser/sync/sessions/sessions_sync_manager_unittest.cc',
             'browser/sync/sync_global_error_unittest.cc',
             'browser/translate/translate_manager_render_view_host_unittest.cc',
-            'browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_unittest.cc',
             'browser/ui/bookmarks/bookmark_context_menu_controller_unittest.cc',
             'browser/ui/bookmarks/bookmark_unittest.cc',
             'browser/ui/browser_command_controller_unittest.cc',