[AndroidSms] Add migration cookie logic.

This CL adds some logic to AndroidSmsAppSetupController so that when
removing the PWA, a cookie will be set on the old domain to indicate
the  status of the migration. This will allow the client to redirect
users to new url. We also attempt to remove this cookie when
installing the PWA so that the client won't redirect inside the PWA
in a rollback case.

Bug: 927472
Change-Id: Ia11ce407b2140242c6746d2b28e39d229dff58d4
Reviewed-on: https://chromium-review.googlesource.com/c/1448984
Commit-Queue: Azeem Arshad <azeemarshad@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628435}

chrome/browser/chromeos/android_sms/android_sms_app_manager_impl.cc

@@ -173,8 +173,9 @@ void AndroidSmsAppManagerImpl::OnSetUpNewAppResult(bool success) {
 
   // Finish the migration by removing the old app now that it has been replaced.
   setup_controller_->RemoveApp(
-      GetAndroidMessagesURLOld(),
-      GetAndroidMessagesURLOld(true /* use_install_url */),
+      GetAndroidMessagesURLOld() /* app_url */,
+      GetAndroidMessagesURLOld(true /* use_install_url */) /* install_url */,
+      GetAndroidMessagesURL() /* migrated_to_app_url */,
       base::BindOnce(&AndroidSmsAppManagerImpl::OnRemoveOldAppResult,
                      weak_ptr_factory_.GetWeakPtr()));
 }

chrome/browser/chromeos/android_sms/android_sms_app_manager_impl_unittest.cc

@@ -294,6 +294,7 @@ TEST_F(AndroidSmsAppManagerImplTest,
       GetAndroidMessagesURLOld() /* expected_app_url */,
       GetAndroidMessagesURLOld(
           true /* use_install_url */) /* expected_install_url */,
+      GetAndroidMessagesURL() /* expected_migrated_to_app_url */,
       true /* success */);
   EXPECT_EQ(1u, test_observer()->num_installed_app_url_changed_events());
 }

chrome/browser/chromeos/android_sms/android_sms_app_setup_controller.h

@@ -25,10 +25,11 @@ class AndroidSmsAppSetupController {
 
   using SuccessCallback = base::OnceCallback<void(bool)>;
 
-  // Performs the setup process for the app at |url|, which includes:
+  // Performs the setup process for the app at |app_url|, which includes:
   //   (1) Installing the PWA,
   //   (2) Granting permission for the PWA to show notifications, and
-  //   (3) Setting a cookie which defaults the PWA to remember this computer.
+  //   (3) Removing previously set migration cookie, if any.
+  //   (4) Setting a cookie which defaults the PWA to remember this computer.
   // The |app_url|  parameter should have the root URL of the app to install
   // and should be the same as the service worker scope
   // The |install_url| parameter is the url to install the app from and cannot
@@ -37,8 +38,8 @@ class AndroidSmsAppSetupController {
                         const GURL& install_url,
                         SuccessCallback callback) = 0;
 
-  // Returns the extension for the PWA at |url|; if no PWA exists, null is
-  // returned.
+  // Returns the extension for the PWA at |install_url|; if no PWA exists, null
+  // is returned.
   virtual const extensions::Extension* GetPwa(const GURL& install_url) = 0;
 
   // Deletes the cookie which causes the PWA to remember this computer by
@@ -49,10 +50,16 @@ class AndroidSmsAppSetupController {
       const GURL& app_url,
       SuccessCallback callback) = 0;
 
-  // Uninstalls the app at |url| and deletes relevant cookies from the setup
-  // process.
+  // Uninstalls the app at |app_url| and deletes relevant cookies from the setup
+  // process. This also sets a migration cookie that allows the client to start
+  // redirecting users to the new domain.
+  // The |app_url| parameter should have the root URL of the app to install
+  // and should be the same as the service worker scope
+  // The |install_url| parameter is the url to that was used to install the app.
+  // The |migrated_to_app_url| should be the url that the PWA was migrated to.
   virtual void RemoveApp(const GURL& app_url,
                          const GURL& install_url,
+                         const GURL& migrated_to_app_url,
                          SuccessCallback callback) = 0;
 
  private:

chrome/browser/chromeos/android_sms/android_sms_app_setup_controller_impl.cc

@@ -32,6 +32,7 @@ namespace android_sms {
 namespace {
 
 const char kDefaultToPersistCookieName[] = "default_to_persist";
+const char kMigrationCookieName[] = "cros_migrated_to";
 const char kDefaultToPersistCookieValue[] = "true";
 
 }  // namespace
@@ -88,7 +89,8 @@ void AndroidSmsAppSetupControllerImpl::SetUpApp(const GURL& app_url,
               false /* http_only */, net::CookieSameSite::STRICT_MODE,
               net::COOKIE_PRIORITY_DEFAULT),
           true /* secure_source */, false /* modify_http_only */,
-          base::BindOnce(&AndroidSmsAppSetupControllerImpl::OnSetCookieResult,
+          base::BindOnce(&AndroidSmsAppSetupControllerImpl::
+                             OnSetRememberDeviceByDefaultCookieResult,
                          weak_ptr_factory_.GetWeakPtr(), app_url, install_url,
                          std::move(callback)));
 }
@@ -111,14 +113,17 @@ void AndroidSmsAppSetupControllerImpl::DeleteRememberDeviceByDefaultCookie(
   pwa_delegate_->GetCookieManager(app_url, profile_)
       ->DeleteCookies(
           std::move(filter),
-          base::BindOnce(
-              &AndroidSmsAppSetupControllerImpl::OnDeleteCookiesResult,
-              weak_ptr_factory_.GetWeakPtr(), app_url, std::move(callback)));
+          base::BindOnce(&AndroidSmsAppSetupControllerImpl::
+                             OnDeleteRememberDeviceByDefaultCookieResult,
+                         weak_ptr_factory_.GetWeakPtr(), app_url,
+                         std::move(callback)));
 }
 
-void AndroidSmsAppSetupControllerImpl::RemoveApp(const GURL& app_url,
-                                                 const GURL& install_url,
-                                                 SuccessCallback callback) {
+void AndroidSmsAppSetupControllerImpl::RemoveApp(
+    const GURL& app_url,
+    const GURL& install_url,
+    const GURL& migrated_to_app_url,
+    SuccessCallback callback) {
   // If there is no app installed at |url|, there is nothing more to do.
   if (!pwa_delegate_->GetPwaForUrl(install_url, profile_)) {
     PA_LOG(VERBOSE) << "AndroidSmsAppSetupControllerImpl::RemoveApp(): No app "
@@ -142,24 +147,45 @@ void AndroidSmsAppSetupControllerImpl::RemoveApp(const GURL& app_url,
       std::vector<GURL>{install_url},
       base::BindRepeating(
           &AndroidSmsAppSetupControllerImpl::OnAppUninstallResult,
-          weak_ptr_factory_.GetWeakPtr(), id, app_url));
+          weak_ptr_factory_.GetWeakPtr(), id, app_url, migrated_to_app_url));
 }
 
-void AndroidSmsAppSetupControllerImpl::OnSetCookieResult(
+void AndroidSmsAppSetupControllerImpl::OnSetRememberDeviceByDefaultCookieResult(
     const GURL& app_url,
     const GURL& install_url,
     SuccessCallback callback,
     bool succeeded) {
   if (!succeeded) {
-    PA_LOG(WARNING) << "AndroidSmsAppSetupControllerImpl::"
-                    << "OnSetCookieResult(): Failed to set "
-                    << "DefaultToPersist cookie at " << app_url
-                    << ". Proceeding with installation request.";
+    PA_LOG(WARNING)
+        << "AndroidSmsAppSetupControllerImpl::"
+        << "OnSetRememberDeviceByDefaultCookieResult(): Failed to set "
+        << "DefaultToPersist cookie at " << app_url << ". Proceeding "
+        << "to remove migration cookie.";
   }
 
+  // Delete migration cookie in case it was set by a previous RemoveApp call.
+  network::mojom::CookieDeletionFilterPtr filter(
+      network::mojom::CookieDeletionFilter::New());
+  filter->url = app_url;
+  filter->cookie_name = kMigrationCookieName;
+  pwa_delegate_->GetCookieManager(app_url, profile_)
+      ->DeleteCookies(
+          std::move(filter),
+          base::BindOnce(
+              &AndroidSmsAppSetupControllerImpl::OnDeleteMigrationCookieResult,
+              weak_ptr_factory_.GetWeakPtr(), app_url, install_url,
+              std::move(callback)));
+}
+
+void AndroidSmsAppSetupControllerImpl::OnDeleteMigrationCookieResult(
+    const GURL& app_url,
+    const GURL& install_url,
+    SuccessCallback callback,
+    uint32_t num_deleted) {
   // If the app is already installed at |url|, there is nothing more to do.
   if (pwa_delegate_->GetPwaForUrl(install_url, profile_)) {
-    PA_LOG(VERBOSE) << "AndroidSmsAppSetupControllerImpl::OnSetCookieResult(): "
+    PA_LOG(VERBOSE) << "AndroidSmsAppSetupControllerImpl::"
+                    << "OnDeleteMigrationCookieResult():"
                     << "App is already installed at " << install_url
                     << "; skipping setup process.";
     std::move(callback).Run(true /* success */);
@@ -216,6 +242,7 @@ void AndroidSmsAppSetupControllerImpl::OnAppInstallResult(
 void AndroidSmsAppSetupControllerImpl::OnAppUninstallResult(
     const base::UnguessableToken& id,
     const GURL& app_url,
+    const GURL& migrated_to_app_url,
     const GURL& install_url,
     bool succeeded) {
   UMA_HISTOGRAM_BOOLEAN("AndroidSms.PWAUninstallationResult", succeeded);
@@ -234,18 +261,49 @@ void AndroidSmsAppSetupControllerImpl::OnAppUninstallResult(
     return;
   }
 
-  DeleteRememberDeviceByDefaultCookie(app_url, std::move(callback));
+  // Set migration cookie on the client for which the PWA was just uninstalled.
+  // The client checks for this cookie to redirect users to the new domain. This
+  // prevents unwanted connection stealing between old and new clients should
+  // the user try to open old client.
+  pwa_delegate_->GetCookieManager(app_url, profile_)
+      ->SetCanonicalCookie(
+          *net::CanonicalCookie::CreateSanitizedCookie(
+              app_url, kMigrationCookieName, migrated_to_app_url.GetContent(),
+              std::string() /* domain */, std::string() /* path */,
+              base::Time::Now() /* creation_time */,
+              base::Time() /* expiration_time */,
+              base::Time::Now() /* last_access_time */, true /* secure */,
+              false /* http_only */, net::CookieSameSite::STRICT_MODE,
+              net::COOKIE_PRIORITY_DEFAULT),
+          true /* secure_source */, false /* modify_http_only */,
+          base::BindOnce(
+              &AndroidSmsAppSetupControllerImpl::OnSetMigrationCookieResult,
+              weak_ptr_factory_.GetWeakPtr(), app_url, std::move(callback)));
 }
 
-void AndroidSmsAppSetupControllerImpl::OnDeleteCookiesResult(
+void AndroidSmsAppSetupControllerImpl::OnSetMigrationCookieResult(
     const GURL& app_url,
     SuccessCallback callback,
-    uint32_t num_deleted) {
+    bool succeeded) {
+  if (!succeeded) {
+    PA_LOG(ERROR)
+        << "AndroidSmsAppSetupControllerImpl::OnSetMigrationCookieResult(): "
+        << "Failed to set migration cookie for " << app_url << ". Proceeding "
+        << "to remove DefaultToPersist cookie.";
+  }
+
+  DeleteRememberDeviceByDefaultCookie(app_url, std::move(callback));
+}
+
+void AndroidSmsAppSetupControllerImpl::
+    OnDeleteRememberDeviceByDefaultCookieResult(const GURL& app_url,
+                                                SuccessCallback callback,
+                                                uint32_t num_deleted) {
   if (num_deleted != 1u) {
     PA_LOG(WARNING) << "AndroidSmsAppSetupControllerImpl::"
-                    << "OnDeleteCookiesResult(): Tried to delete a single "
-                    << "cookie at " << app_url << ", but " << num_deleted << " "
-                    << "cookies were deleted.";
+                    << "OnDeleteRememberDeviceByDefaultCookieResult(): "
+                    << "Tried to delete a single cookie at " << app_url
+                    << ", but " << num_deleted << " cookies were deleted.";
   }
 
   // Even if an unexpected number of cookies was deleted, consider this a

chrome/browser/chromeos/android_sms/android_sms_app_setup_controller_impl.h

@@ -62,23 +62,33 @@ class AndroidSmsAppSetupControllerImpl : public AndroidSmsAppSetupController {
                                            SuccessCallback callback) override;
   void RemoveApp(const GURL& app_url,
                  const GURL& install_url,
+                 const GURL& migrated_to_app_url,
                  SuccessCallback callback) override;
 
-  void OnSetCookieResult(const GURL& app_url,
-                         const GURL& install_url,
-                         SuccessCallback callback,
-                         bool succeeded);
+  void OnSetRememberDeviceByDefaultCookieResult(const GURL& app_url,
+                                                const GURL& install_url,
+                                                SuccessCallback callback,
+                                                bool succeeded);
+  void OnSetMigrationCookieResult(const GURL& app_url,
+                                  SuccessCallback callback,
+                                  bool succeeded);
+
   void OnAppInstallResult(SuccessCallback callback,
                           const GURL& app_url,
                           const GURL& install_url,
                           web_app::InstallResultCode code);
   void OnAppUninstallResult(const base::UnguessableToken& id,
                             const GURL& app_url,
+                            const GURL& migrated_to_app_url,
                             const GURL& install_url,
                             bool succeeded);
-  void OnDeleteCookiesResult(const GURL& app_url,
-                             SuccessCallback callback,
-                             uint32_t num_deleted);
+  void OnDeleteRememberDeviceByDefaultCookieResult(const GURL& app_url,
+                                                   SuccessCallback callback,
+                                                   uint32_t num_deleted);
+  void OnDeleteMigrationCookieResult(const GURL& app_url,
+                                     const GURL& install_url,
+                                     SuccessCallback callback,
+                                     uint32_t num_deleted);
 
   void SetPwaDelegateForTesting(std::unique_ptr<PwaDelegate> test_pwa_delegate);
 

chrome/browser/chromeos/android_sms/android_sms_app_setup_controller_impl_unittest.cc

@@ -59,6 +59,7 @@ class FakeCookieManager : public network::mojom::CookieManager {
 
   void InvokePendingSetCanonicalCookieCallback(
       const std::string& expected_cookie_name,
+      const std::string& expected_cookie_value,
       bool expected_secure_source,
       bool expected_modify_http_only,
       bool success) {
@@ -67,6 +68,7 @@ class FakeCookieManager : public network::mojom::CookieManager {
     set_canonical_cookie_calls_.erase(set_canonical_cookie_calls_.begin());
 
     EXPECT_EQ(expected_cookie_name, std::get<0>(params).Name());
+    EXPECT_EQ(expected_cookie_value, std::get<0>(params).Value());
     EXPECT_EQ(expected_secure_source, std::get<1>(params));
     EXPECT_EQ(expected_modify_http_only, std::get<2>(params));
 
@@ -223,9 +225,13 @@ class AndroidSmsAppSetupControllerImplTest : public testing::Test {
 
     fake_cookie_manager_->InvokePendingSetCanonicalCookieCallback(
         "default_to_persist" /* expected_cookie_name */,
-        true /* expected_secure_source */,
+        "true" /* expected_cookie_value */, true /* expected_secure_source */,
         false /* expected_modify_http_only */, true /* success */);
 
+    fake_cookie_manager_->InvokePendingDeleteCookiesCallback(
+        app_url, "cros_migrated_to" /* expected_cookie_name */,
+        true /* success */);
+
     // If the PWA was not already installed at the URL, SetUpApp() should
     // install it.
     if (!test_pwa_delegate_->GetPwaForUrl(install_url, &profile_)) {
@@ -267,6 +273,7 @@ class AndroidSmsAppSetupControllerImplTest : public testing::Test {
 
   void CallRemoveApp(const GURL& app_url,
                      const GURL& install_url,
+                     const GURL& migrated_to_app_url,
                      size_t num_expected_app_uninstalls) {
     const auto& uninstall_requests =
         test_pending_app_manager_->uninstall_requests();
@@ -276,7 +283,7 @@ class AndroidSmsAppSetupControllerImplTest : public testing::Test {
     base::HistogramTester histogram_tester;
 
     setup_controller_->RemoveApp(
-        app_url, install_url,
+        app_url, install_url, migrated_to_app_url,
         base::BindOnce(&AndroidSmsAppSetupControllerImplTest::OnRemoveAppResult,
                        base::Unretained(this), run_loop.QuitClosure()));
 
@@ -287,6 +294,12 @@ class AndroidSmsAppSetupControllerImplTest : public testing::Test {
                 uninstall_requests.size());
       EXPECT_EQ(install_url, uninstall_requests.back());
 
+      fake_cookie_manager_->InvokePendingSetCanonicalCookieCallback(
+          "cros_migrated_to" /* expected_cookie_name */,
+          migrated_to_app_url.GetContent() /* expected_cookie_value */,
+          true /* expected_secure_source */,
+          false /* expected_modify_http_only */, true /* success */);
+
       fake_cookie_manager_->InvokePendingDeleteCookiesCallback(
           app_url, "default_to_persist" /* expected_cookie_name */,
           true /* success */);
@@ -381,6 +394,7 @@ TEST_F(AndroidSmsAppSetupControllerImplTest, SetUpAppThenRemove) {
                1u /* num_expected_app_installs */);
   test_pwa_delegate()->SetHasPwa(GURL(kTestInstallUrl1), true);
   CallRemoveApp(GURL(kTestUrl1), GURL(kTestInstallUrl1),
+                GURL(kTestUrl2) /* migrated_to_app_url */,
                 1u /* num_expected_app_uninstalls */);
   test_pwa_delegate()->SetHasPwa(GURL(kTestInstallUrl1), false);
 
@@ -389,6 +403,7 @@ TEST_F(AndroidSmsAppSetupControllerImplTest, SetUpAppThenRemove) {
                1u /* num_expected_app_installs */);
   test_pwa_delegate()->SetHasPwa(GURL(kTestInstallUrl1), true);
   CallRemoveApp(GURL(kTestUrl1), GURL(kTestInstallUrl1),
+                GURL(kTestUrl2) /* migrated_to_app_url */,
                 1u /* num_expected_app_uninstalls */);
   test_pwa_delegate()->SetHasPwa(GURL(kTestInstallUrl1), false);
 }
@@ -396,6 +411,7 @@ TEST_F(AndroidSmsAppSetupControllerImplTest, SetUpAppThenRemove) {
 TEST_F(AndroidSmsAppSetupControllerImplTest, RemoveApp_NoInstalledApp) {
   // Do not have an installed app before attempting to remove it.
   CallRemoveApp(GURL(kTestUrl1), GURL(kTestInstallUrl1),
+                GURL(kTestUrl2) /* migrated_to_app_url */,
                 0u /* num_expected_app_uninstalls */);
 }
 

chrome/browser/chromeos/android_sms/fake_android_sms_app_setup_controller.cc

@@ -96,6 +96,7 @@ void FakeAndroidSmsAppSetupController::CompletePendingDeleteCookieRequest(
 void FakeAndroidSmsAppSetupController::CompleteRemoveAppRequest(
     const GURL& expected_app_url,
     const GURL& expected_install_url,
+    const GURL& expected_migrated_to_app_url,
     bool should_succeed) {
   DCHECK(!pending_remove_app_requests_.empty());
 
@@ -103,17 +104,18 @@ void FakeAndroidSmsAppSetupController::CompleteRemoveAppRequest(
   pending_remove_app_requests_.erase(pending_remove_app_requests_.begin());
   DCHECK_EQ(expected_app_url, std::get<0>(*request));
   DCHECK_EQ(expected_install_url, std::get<1>(*request));
+  DCHECK_EQ(expected_migrated_to_app_url, std::get<2>(*request));
 
   if (should_succeed)
     SetAppAtUrl(expected_install_url, base::nullopt /* id_for_app */);
 
-  std::move(std::get<2>(*request)).Run(should_succeed);
+  std::move(std::get<3>(*request)).Run(should_succeed);
 }
 
 void FakeAndroidSmsAppSetupController::SetUpApp(const GURL& app_url,
                                                 const GURL& install_url,
                                                 SuccessCallback callback) {
-  pending_set_up_app_requests_.push_back(std::make_unique<AppRequestData>(
+  pending_set_up_app_requests_.push_back(std::make_unique<SetUpAppData>(
       app_url, install_url, std::move(callback)));
 }
 
@@ -129,14 +131,16 @@ void FakeAndroidSmsAppSetupController::DeleteRememberDeviceByDefaultCookie(
     const GURL& app_url,
     SuccessCallback callback) {
   pending_delete_cookie_requests_.push_back(
-      std::make_unique<RequestData>(app_url, std::move(callback)));
+      std::make_unique<DeleteCookieData>(app_url, std::move(callback)));
 }
 
-void FakeAndroidSmsAppSetupController::RemoveApp(const GURL& app_url,
-                                                 const GURL& install_url,
-                                                 SuccessCallback callback) {
-  pending_remove_app_requests_.push_back(std::make_unique<AppRequestData>(
-      app_url, install_url, std::move(callback)));
+void FakeAndroidSmsAppSetupController::RemoveApp(
+    const GURL& app_url,
+    const GURL& install_url,
+    const GURL& migrated_to_app_url,
+    SuccessCallback callback) {
+  pending_remove_app_requests_.push_back(std::make_unique<RemoveAppData>(
+      app_url, install_url, migrated_to_app_url, std::move(callback)));
 }
 
 }  // namespace android_sms

chrome/browser/chromeos/android_sms/fake_android_sms_app_setup_controller.h

@@ -67,6 +67,7 @@ class FakeAndroidSmsAppSetupController : public AndroidSmsAppSetupController {
   // app will remain in place.
   void CompleteRemoveAppRequest(const GURL& expected_app_url,
                                 const GURL& expected_install_url,
+                                const GURL& expected_migrated_to_app_url,
                                 bool success);
 
  private:
@@ -79,14 +80,17 @@ class FakeAndroidSmsAppSetupController : public AndroidSmsAppSetupController {
                                            SuccessCallback callback) override;
   void RemoveApp(const GURL& app_url,
                  const GURL& install_url,
+                 const GURL& migrated_to_app_url,
                  SuccessCallback callback) override;
 
-  using AppRequestData = std::tuple<GURL, GURL, SuccessCallback>;
-  using RequestData = std::pair<GURL, SuccessCallback>;
+  using SetUpAppData = std::tuple<GURL, GURL, SuccessCallback>;
+  std::list<std::unique_ptr<SetUpAppData>> pending_set_up_app_requests_;
 
-  std::list<std::unique_ptr<RequestData>> pending_delete_cookie_requests_;
-  std::list<std::unique_ptr<AppRequestData>> pending_set_up_app_requests_;
-  std::list<std::unique_ptr<AppRequestData>> pending_remove_app_requests_;
+  using RemoveAppData = std::tuple<GURL, GURL, GURL, SuccessCallback>;
+  std::list<std::unique_ptr<RemoveAppData>> pending_remove_app_requests_;
+
+  using DeleteCookieData = std::pair<GURL, SuccessCallback>;
+  std::list<std::unique_ptr<DeleteCookieData>> pending_delete_cookie_requests_;
 
   base::flat_map<GURL, AppMetadata> install_url_to_metadata_map_;