[iOS] Display consistency promo on foreground of accounts.google.com.

We display the account consistency promo on top of Google web sign-in to
allow users to dismiss the promo and only sign-in on the web.

Bug: 1125631
Change-Id: Ife6da5203a3bff8e72fe79a9be06cefd902cc5ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2416376
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Ali Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811665}

components/signin/ios/browser/account_consistency_service.mm

@@ -26,6 +26,8 @@
 #include "ios/web/common/web_view_creation_util.h"
 #include "ios/web/public/browser_state.h"
 #import "ios/web/public/navigation/web_state_policy_decider.h"
+#import "ios/web/public/web_state.h"
+#include "ios/web/public/web_state_observer.h"
 #include "net/base/mac/url_conversions.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/cookies/canonical_cookie.h"
@@ -63,14 +65,22 @@ static std::string GetDomainFromUrl(const GURL& url) {
 // reacting on the X-Chrome-Manage-Accounts header and notifying its delegate.
 // It also notifies the AccountConsistencyService of domains it should add the
 // CHROME_CONNECTED cookie to.
-class AccountConsistencyHandler : public web::WebStatePolicyDecider {
+class AccountConsistencyHandler : public web::WebStatePolicyDecider,
+                                  public web::WebStateObserver {
  public:
   AccountConsistencyHandler(web::WebState* web_state,
                             AccountConsistencyService* service,
                             AccountReconcilor* account_reconcilor,
                             id<ManageAccountsDelegate> delegate);
 
+  void WebStateDestroyed(web::WebState* web_state) override;
+
  private:
+  // web::WebStateObserver override.
+  void PageLoaded(
+      web::WebState* web_state,
+      web::PageLoadCompletionStatus load_completion_status) override;
+
   // web::WebStatePolicyDecider override.
   // Decides on navigation corresponding to |response| whether the navigation
   // should continue and updates authentication cookies on Google domains.
@@ -80,6 +90,7 @@ class AccountConsistencyHandler : public web::WebStatePolicyDecider {
       base::OnceCallback<void(PolicyDecision)> callback) override;
   void WebStateDestroyed() override;
 
+  bool show_consistency_promo_ = false;
   AccountConsistencyService* account_consistency_service_;  // Weak.
   AccountReconcilor* account_reconcilor_;                   // Weak.
   __weak id<ManageAccountsDelegate> delegate_;
@@ -94,7 +105,9 @@ AccountConsistencyHandler::AccountConsistencyHandler(
     : web::WebStatePolicyDecider(web_state),
       account_consistency_service_(service),
       account_reconcilor_(account_reconcilor),
-      delegate_(delegate) {}
+      delegate_(delegate) {
+  web_state->AddObserver(this);
+}
 
 void AccountConsistencyHandler::ShouldAllowResponse(
     NSURLResponse* response,
@@ -134,6 +147,11 @@ void AccountConsistencyHandler::ShouldAllowResponse(
       base::SysNSStringToUTF8(manage_accounts_header));
 
   account_reconcilor_->OnReceivedManageAccountsResponse(params.service_type);
+  // Reset boolean that tracks displaying the sign-in consistency promo. This
+  // ensures that the promo is cancelled once navigation has started and the
+  // WKWebView is cancelling previous navigations.
+  show_consistency_promo_ = false;
+
   switch (params.service_type) {
     case signin::GAIA_SERVICE_TYPE_INCOGNITO: {
       GURL continue_url = GURL(params.continue_url);
@@ -145,7 +163,12 @@ void AccountConsistencyHandler::ShouldAllowResponse(
     case signin::GAIA_SERVICE_TYPE_SIGNUP:
     case signin::GAIA_SERVICE_TYPE_ADDSESSION:
       if (params.show_consistency_promo) {
-        [delegate_ onShowConsistencyPromo];
+        show_consistency_promo_ = true;
+        // Allows the URL response to load before showing the consistency promo.
+        // The promo should always be displayed in the foreground of Gaia
+        // sign-on.
+        std::move(callback).Run(PolicyDecision::Allow());
+        return;
       } else {
         [delegate_ onAddAccount];
       }
@@ -169,6 +192,20 @@ void AccountConsistencyHandler::ShouldAllowResponse(
   std::move(callback).Run(PolicyDecision::Cancel());
 }
 
+void AccountConsistencyHandler::PageLoaded(
+    web::WebState* web_state,
+    web::PageLoadCompletionStatus load_completion_status) {
+  if (!show_consistency_promo_ ||
+      !gaia::IsGaiaSignonRealm(web_state->GetLastCommittedURL().GetOrigin()) ||
+      load_completion_status == web::PageLoadCompletionStatus::FAILURE) {
+    return;
+  }
+  [delegate_ onShowConsistencyPromo];
+  show_consistency_promo_ = false;
+}
+
+void AccountConsistencyHandler::WebStateDestroyed(web::WebState* web_state) {}
+
 void AccountConsistencyHandler::WebStateDestroyed() {
   account_consistency_service_->RemoveWebStateHandler(web_state());
 }
@@ -222,6 +259,8 @@ void AccountConsistencyService::SetWebStateHandler(
 void AccountConsistencyService::RemoveWebStateHandler(
     web::WebState* web_state) {
   DCHECK_LT(0u, web_state_handlers_.count(web_state));
+  web_state->RemoveObserver(
+      (AccountConsistencyHandler*)web_state_handlers_[web_state].get());
   web_state_handlers_.erase(web_state);
 }
 

components/signin/ios/browser/account_consistency_service_unittest.mm

@@ -276,6 +276,13 @@ class AccountConsistencyServiceTest : public PlatformTest {
                        /*domain=*/std::string()));
   }
 
+  // Simulate navigating to a URL with the given page load completion status.
+  void SimulateNavigateToUrl(web::PageLoadCompletionStatus status,
+                             const GURL& url) {
+    web_state_.SetCurrentURL(url);
+    web_state_.OnPageLoaded(status);
+  }
+
   // Simulate the action of the action GaiaCookieManagerService to cleanup
   // the cookies once the sign-out is done.
   void SimulateGaiaCookieManagerServiceLogout() {
@@ -467,8 +474,111 @@ TEST_F(AccountConsistencyServiceTest,
   EXPECT_CALL(*account_reconcilor_, OnReceivedManageAccountsResponse(
                                         signin::GAIA_SERVICE_TYPE_ADDSESSION))
       .Times(1);
-  EXPECT_FALSE(
+  EXPECT_TRUE(
+      web_state_.ShouldAllowResponse(response, /* for_main_frame = */ true));
+  SimulateNavigateToUrl(web::PageLoadCompletionStatus::SUCCESS,
+                        GURL("https://accounts.google.com/"));
+  web_state_.WebStateDestroyed();
+
+  EXPECT_OCMOCK_VERIFY(delegate);
+}
+
+// Tests that the consistency promo is not displayed when a page fails to load.
+TEST_F(AccountConsistencyServiceTest,
+       ChromeManageAccountsNotShowConsistencyPromoOnPageLoadFailure) {
+  id delegate =
+      [OCMockObject mockForProtocol:@protocol(ManageAccountsDelegate)];
+  [[delegate reject] onShowConsistencyPromo];
+
+  NSDictionary* headers = [NSDictionary
+      dictionaryWithObject:@"action=ADDSESSION,show_consistency_promo=true"
+                    forKey:@"X-Chrome-Manage-Accounts"];
+  NSHTTPURLResponse* response = [[NSHTTPURLResponse alloc]
+       initWithURL:[NSURL URLWithString:@"https://accounts.google.com/"]
+        statusCode:200
+       HTTPVersion:@"HTTP/1.1"
+      headerFields:headers];
+  account_consistency_service_->SetWebStateHandler(&web_state_, delegate);
+  EXPECT_CALL(*account_reconcilor_, OnReceivedManageAccountsResponse(
+                                        signin::GAIA_SERVICE_TYPE_ADDSESSION))
+      .Times(1);
+  EXPECT_TRUE(
+      web_state_.ShouldAllowResponse(response, /* for_main_frame = */ true));
+  SimulateNavigateToUrl(web::PageLoadCompletionStatus::FAILURE,
+                        GURL("https://accounts.google.com/"));
+  web_state_.WebStateDestroyed();
+
+  EXPECT_OCMOCK_VERIFY(delegate);
+}
+
+// Tests that the consistency promo is not displayed when a page fails to load
+// and user chooses another action.
+TEST_F(AccountConsistencyServiceTest,
+       ChromeManageAccountsNotShowConsistencyPromoOnPageLoadFailureRedirect) {
+  id delegate =
+      [OCMockObject mockForProtocol:@protocol(ManageAccountsDelegate)];
+  [[delegate expect] onAddAccount];
+  [[delegate reject] onShowConsistencyPromo];
+
+  NSDictionary* headers = [NSDictionary
+      dictionaryWithObject:@"action=ADDSESSION,show_consistency_promo=true"
+                    forKey:@"X-Chrome-Manage-Accounts"];
+  account_consistency_service_->SetWebStateHandler(&web_state_, delegate);
+  EXPECT_CALL(*account_reconcilor_, OnReceivedManageAccountsResponse(
+                                        signin::GAIA_SERVICE_TYPE_ADDSESSION))
+      .Times(2);
+
+  NSHTTPURLResponse* responseSignin = [[NSHTTPURLResponse alloc]
+       initWithURL:[NSURL URLWithString:@"https://accounts.google.com/"]
+        statusCode:200
+       HTTPVersion:@"HTTP/1.1"
+      headerFields:headers];
+  EXPECT_TRUE(web_state_.ShouldAllowResponse(responseSignin,
+                                             /* for_main_frame = */ true));
+  const GURL accountsUrl = GURL("https://accounts.google.com/");
+  SimulateNavigateToUrl(web::PageLoadCompletionStatus::FAILURE, accountsUrl);
+
+  NSDictionary* headersAddAccount =
+      [NSDictionary dictionaryWithObject:@"action=ADDSESSION"
+                                  forKey:@"X-Chrome-Manage-Accounts"];
+  NSHTTPURLResponse* responseAddAccount = [[NSHTTPURLResponse alloc]
+       initWithURL:[NSURL URLWithString:@"https://accounts.google.com/"]
+        statusCode:200
+       HTTPVersion:@"HTTP/1.1"
+      headerFields:headersAddAccount];
+  EXPECT_FALSE(web_state_.ShouldAllowResponse(responseAddAccount,
+                                              /* for_main_frame = */ true));
+  SimulateNavigateToUrl(web::PageLoadCompletionStatus::SUCCESS, accountsUrl);
+
+  web_state_.WebStateDestroyed();
+
+  EXPECT_OCMOCK_VERIFY(delegate);
+}
+
+// Tests that the consistency promo is not displayed when a non GAIA URL is
+// committed.
+TEST_F(AccountConsistencyServiceTest,
+       ChromeManageAccountsNotShowConsistencyPromoOnNonGaiaURL) {
+  id delegate =
+      [OCMockObject mockForProtocol:@protocol(ManageAccountsDelegate)];
+  [[delegate reject] onShowConsistencyPromo];
+
+  NSDictionary* headers = [NSDictionary
+      dictionaryWithObject:@"action=ADDSESSION,show_consistency_promo=true"
+                    forKey:@"X-Chrome-Manage-Accounts"];
+  NSHTTPURLResponse* response = [[NSHTTPURLResponse alloc]
+       initWithURL:[NSURL URLWithString:@"https://accounts.google.com/"]
+        statusCode:200
+       HTTPVersion:@"HTTP/1.1"
+      headerFields:headers];
+  account_consistency_service_->SetWebStateHandler(&web_state_, delegate);
+  EXPECT_CALL(*account_reconcilor_, OnReceivedManageAccountsResponse(
+                                        signin::GAIA_SERVICE_TYPE_ADDSESSION))
+      .Times(1);
+  EXPECT_TRUE(
       web_state_.ShouldAllowResponse(response, /* for_main_frame = */ true));
+  SimulateNavigateToUrl(web::PageLoadCompletionStatus::SUCCESS,
+                        GURL("https://youtube.com/"));
   web_state_.WebStateDestroyed();
 
   EXPECT_OCMOCK_VERIFY(delegate);
@@ -680,5 +790,6 @@ TEST_F(AccountConsistencyServiceTest,
       headerFields:headers];
   EXPECT_TRUE(web_state_.ShouldAllowResponse(responseGaia,
                                              /* for_main_frame = */ true));
+  web_state_.WebStateDestroyed();
   CheckDomainHasChromeConnectedCookie("google.com");
 }

ios/chrome/browser/ui/browser_view/browser_view_controller_unittest.mm

@@ -178,10 +178,6 @@ class BrowserViewControllerTest : public BlockCleanupTest {
     [[bvc_ view] removeFromSuperview];
     [bvc_ shutdown];
 
-    // Cleanup to avoid debugger crash in non empty observer lists.
-    browser_->GetWebStateList()->CloseAllWebStates(
-        WebStateList::ClosingFlags::CLOSE_NO_FLAGS);
-
     BlockCleanupTest::TearDown();
   }