Clear GCM data when the user clears cookies and other site data This is needed for the work to drop GCM sign-in enforcement. Since we no longer wipe out the GCM data when the user signs out, we need to give user a chance to clear the data if he or she wants. BUG=384041 TEST=Manual test by selecting "Cookies and other site and plugin data" Review URL: https://codereview.chromium.org/562423002 Cr-Commit-Position: refs/heads/master@{#294754}
diff --git a/chrome/browser/browsing_data/browsing_data_remover.cc b/chrome/browser/browsing_data/browsing_data_remover.cc index 401a633..a38415b 100644 --- a/chrome/browser/browsing_data/browsing_data_remover.cc +++ b/chrome/browser/browsing_data/browsing_data_remover.cc
@@ -35,6 +35,8 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/safe_browsing/safe_browsing_service.h" #include "chrome/browser/search_engines/template_url_service_factory.h" +#include "chrome/browser/services/gcm/gcm_profile_service.h" +#include "chrome/browser/services/gcm/gcm_profile_service_factory.h" #include "chrome/browser/sessions/session_service.h" #include "chrome/browser/sessions/session_service_factory.h" #include "chrome/browser/sessions/tab_restore_service.h" @@ -45,6 +47,7 @@ #include "components/autofill/core/browser/personal_data_manager.h" #include "components/autofill/core/browser/webdata/autofill_webdata_service.h" #include "components/domain_reliability/service.h" +#include "components/gcm_driver/gcm_driver.h" #include "components/nacl/browser/nacl_browser.h" #include "components/nacl/browser/pnacl_host.h" #include "components/password_manager/core/browser/password_store.h" @@ -532,6 +535,15 @@ } #endif +#if !defined(OS_ANDROID) + if (remove_mask & REMOVE_GCM) { + gcm::GCMProfileService* gcm_profile_service = + gcm::GCMProfileServiceFactory::GetForProfile(profile_); + if (gcm_profile_service) + gcm_profile_service->driver()->Purge(); + } +#endif + if (remove_mask & REMOVE_PASSWORDS) { content::RecordAction(UserMetricsAction("ClearBrowsingData_Passwords")); password_manager::PasswordStore* password_store =
diff --git a/chrome/browser/browsing_data/browsing_data_remover.h b/chrome/browser/browsing_data/browsing_data_remover.h index 7a2b063..3871455 100644 --- a/chrome/browser/browsing_data/browsing_data_remover.h +++ b/chrome/browser/browsing_data/browsing_data_remover.h
@@ -93,6 +93,9 @@ #if defined(OS_ANDROID) REMOVE_APP_BANNER_DATA = 1 << 15, #endif +#if !defined(OS_ANDROID) + REMOVE_GCM = 1 << 16, +#endif // The following flag is used only in tests. In normal usage, hosted app // data is controlled by the REMOVE_COOKIES flag, applied to the // protected-web origin. @@ -109,6 +112,9 @@ #if defined(OS_ANDROID) REMOVE_APP_BANNER_DATA | #endif +#if !defined(OS_ANDROID) + REMOVE_GCM | +#endif REMOVE_CHANNEL_IDS, // Includes all the available remove options. Meant to be used by clients
diff --git a/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc b/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc index 5d017ea..43e0e1f 100644 --- a/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc +++ b/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc
@@ -496,39 +496,41 @@ // Test cookie and app data settings. IN_PROC_BROWSER_TEST_F(ExtensionBrowsingDataTest, SettingsFunctionSiteData) { - int site_data_no_plugins = BrowsingDataRemover::REMOVE_SITE_DATA & - ~BrowsingDataRemover::REMOVE_PLUGIN_DATA; + int site_data_no_plugins_and_gcm = BrowsingDataRemover::REMOVE_SITE_DATA & + ~BrowsingDataRemover::REMOVE_PLUGIN_DATA & + ~BrowsingDataRemover::REMOVE_GCM; SetPrefsAndVerifySettings(BrowsingDataRemover::REMOVE_COOKIES, UNPROTECTED_WEB, - site_data_no_plugins); + site_data_no_plugins_and_gcm); SetPrefsAndVerifySettings( BrowsingDataRemover::REMOVE_HOSTED_APP_DATA_TESTONLY, PROTECTED_WEB, - site_data_no_plugins); + site_data_no_plugins_and_gcm); SetPrefsAndVerifySettings( BrowsingDataRemover::REMOVE_COOKIES | BrowsingDataRemover::REMOVE_HOSTED_APP_DATA_TESTONLY, PROTECTED_WEB | UNPROTECTED_WEB, - site_data_no_plugins); + site_data_no_plugins_and_gcm); SetPrefsAndVerifySettings( BrowsingDataRemover::REMOVE_COOKIES | BrowsingDataRemover::REMOVE_PLUGIN_DATA, UNPROTECTED_WEB, - BrowsingDataRemover::REMOVE_SITE_DATA); + site_data_no_plugins_and_gcm | BrowsingDataRemover::REMOVE_PLUGIN_DATA); } // Test an arbitrary assortment of settings. IN_PROC_BROWSER_TEST_F(ExtensionBrowsingDataTest, SettingsFunctionAssorted) { - int site_data_no_plugins = BrowsingDataRemover::REMOVE_SITE_DATA & - ~BrowsingDataRemover::REMOVE_PLUGIN_DATA; + int site_data_no_plugins_and_gcm = BrowsingDataRemover::REMOVE_SITE_DATA & + ~BrowsingDataRemover::REMOVE_PLUGIN_DATA & + ~BrowsingDataRemover::REMOVE_GCM; SetPrefsAndVerifySettings( BrowsingDataRemover::REMOVE_COOKIES | BrowsingDataRemover::REMOVE_HISTORY | BrowsingDataRemover::REMOVE_DOWNLOADS, UNPROTECTED_WEB, - site_data_no_plugins | + site_data_no_plugins_and_gcm | BrowsingDataRemover::REMOVE_HISTORY | BrowsingDataRemover::REMOVE_DOWNLOADS); }
diff --git a/chrome/browser/services/gcm/gcm_profile_service.cc b/chrome/browser/services/gcm/gcm_profile_service.cc index 0539acc..069862d 100644 --- a/chrome/browser/services/gcm/gcm_profile_service.cc +++ b/chrome/browser/services/gcm/gcm_profile_service.cc
@@ -116,9 +116,11 @@ // Check is necessary to not crash browser_tests. if (gcm_account_tracker_) gcm_account_tracker_->Stop(); - // TODO(fgorski): If we purge here, what should happen when we get - // OnActiveAccountLogin() right after that? - driver_->Purge(); + // When sign-in enforcement is not dropped, OnSignedOut will also clear all + // the GCM data and a new GCM ID will be retrieved after the user signs in + // again. Otherwise, the user sign-out will not affect the existing GCM + // data. + driver_->OnSignedOut(); } std::string GCMProfileService::IdentityObserver::SignedInUserName() const {
diff --git a/components/gcm_driver/fake_gcm_driver.cc b/components/gcm_driver/fake_gcm_driver.cc index 85cc920..7ee53c5 100644 --- a/components/gcm_driver/fake_gcm_driver.cc +++ b/components/gcm_driver/fake_gcm_driver.cc
@@ -25,6 +25,9 @@ void FakeGCMDriver::OnSignedIn() { } +void FakeGCMDriver::OnSignedOut() { +} + void FakeGCMDriver::Purge() { }
diff --git a/components/gcm_driver/fake_gcm_driver.h b/components/gcm_driver/fake_gcm_driver.h index a549e1f..30a2117 100644 --- a/components/gcm_driver/fake_gcm_driver.h +++ b/components/gcm_driver/fake_gcm_driver.h
@@ -22,6 +22,7 @@ GCMAppHandler* handler) OVERRIDE; virtual void RemoveAppHandler(const std::string& app_id) OVERRIDE; virtual void OnSignedIn() OVERRIDE; + virtual void OnSignedOut() OVERRIDE; virtual void Purge() OVERRIDE; virtual void AddConnectionObserver(GCMConnectionObserver* observer) OVERRIDE; virtual void RemoveConnectionObserver(
diff --git a/components/gcm_driver/gcm_driver.h b/components/gcm_driver/gcm_driver.h index c10d9f1..9878068 100644 --- a/components/gcm_driver/gcm_driver.h +++ b/components/gcm_driver/gcm_driver.h
@@ -72,9 +72,9 @@ // been called, no other GCMDriver methods may be used. virtual void Shutdown(); - // Call this method when the user signs in to a GAIA account. - // TODO(jianli): To be removed when sign-in enforcement is dropped. + // Called when the user signs in to or out of a GAIA account. virtual void OnSignedIn() = 0; + virtual void OnSignedOut() = 0; // Removes all the cached and persisted GCM data. If the GCM service is // restarted after the purge, a new Android ID will be obtained.
diff --git a/components/gcm_driver/gcm_driver_android.cc b/components/gcm_driver/gcm_driver_android.cc index d855363..69da4c5 100644 --- a/components/gcm_driver/gcm_driver_android.cc +++ b/components/gcm_driver/gcm_driver_android.cc
@@ -95,6 +95,9 @@ void GCMDriverAndroid::OnSignedIn() { } +void GCMDriverAndroid::OnSignedOut() { +} + void GCMDriverAndroid::Purge() { }
diff --git a/components/gcm_driver/gcm_driver_android.h b/components/gcm_driver/gcm_driver_android.h index e862c60..5cc97f6 100644 --- a/components/gcm_driver/gcm_driver_android.h +++ b/components/gcm_driver/gcm_driver_android.h
@@ -45,6 +45,7 @@ // GCMDriver implementation: virtual void OnSignedIn() OVERRIDE; + virtual void OnSignedOut() OVERRIDE; virtual void Purge() OVERRIDE; virtual void Enable() OVERRIDE; virtual void AddConnectionObserver(GCMConnectionObserver* observer) OVERRIDE;
diff --git a/components/gcm_driver/gcm_driver_desktop.cc b/components/gcm_driver/gcm_driver_desktop.cc index 6722966..8d35c2c 100644 --- a/components/gcm_driver/gcm_driver_desktop.cc +++ b/components/gcm_driver/gcm_driver_desktop.cc
@@ -369,13 +369,18 @@ EnsureStarted(); } +void GCMDriverDesktop::OnSignedOut() { + signed_in_ = false; + + // When sign-in enforcement is dropped, we will no longer wipe out the GCM + // data when the user signs out. + if (!GCMDriver::IsAllowedForAllUsers()) + Purge(); +} + void GCMDriverDesktop::Purge() { DCHECK(ui_thread_->RunsTasksOnCurrentThread()); - // We still proceed with the check-out logic even if the check-in is not - // initiated in the current session. This will make sure that all the - // persisted data written previously will get purged. - signed_in_ = false; RemoveCachedData(); io_thread_->PostTask(FROM_HERE, @@ -616,7 +621,6 @@ if (app_handlers().empty()) return GCMClient::UNKNOWN_ERROR; - // TODO(jianli): To be removed when sign-in enforcement is dropped. if (!signed_in_ && !GCMDriver::IsAllowedForAllUsers()) return GCMClient::NOT_SIGNED_IN;
diff --git a/components/gcm_driver/gcm_driver_desktop.h b/components/gcm_driver/gcm_driver_desktop.h index 3199a48..51a9845 100644 --- a/components/gcm_driver/gcm_driver_desktop.h +++ b/components/gcm_driver/gcm_driver_desktop.h
@@ -54,6 +54,7 @@ // GCMDriver overrides: virtual void Shutdown() OVERRIDE; virtual void OnSignedIn() OVERRIDE; + virtual void OnSignedOut() OVERRIDE; virtual void Purge() OVERRIDE; virtual void AddAppHandler(const std::string& app_id, GCMAppHandler* handler) OVERRIDE;
diff --git a/components/gcm_driver/gcm_driver_desktop_unittest.cc b/components/gcm_driver/gcm_driver_desktop_unittest.cc index cec87ef..ad95f86 100644 --- a/components/gcm_driver/gcm_driver_desktop_unittest.cc +++ b/components/gcm_driver/gcm_driver_desktop_unittest.cc
@@ -258,7 +258,7 @@ } void GCMDriverTest::SignOut() { - driver_->Purge(); + driver_->OnSignedOut(); PumpIOLoop(); PumpUILoop(); } @@ -350,6 +350,7 @@ } TEST_F(GCMDriverTest, CreateByFieldTrial) { + // Turn on the signal to drop sign-in enforcement. ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial("GCM", "Enabled")); // Create GCMDriver first. GCM is not started. @@ -364,6 +365,18 @@ PumpIOLoop(); EXPECT_TRUE(driver()->IsConnected()); EXPECT_TRUE(gcm_connection_observer()->connected()); + + // Sign-in will not affect GCM state. + SignIn(kTestAccountID1); + PumpIOLoop(); + EXPECT_TRUE(driver()->IsStarted()); + EXPECT_TRUE(driver()->IsConnected()); + + // Sign-out will not affect GCM state. + SignOut(); + PumpIOLoop(); + EXPECT_TRUE(driver()->IsStarted()); + EXPECT_TRUE(driver()->IsConnected()); } TEST_F(GCMDriverTest, Shutdown) {