Fixed crash in CloudPolicyManager.

The ComponentCloudPolicyService is created whenever a user signs in, but isn't
cleared when the user signs out; signing out and back in leads to this crash:

[2774:2774:1126/144024:FATAL:cloud_policy_manager.cc(111)] Check failed: !component_policy_service_.
[0x7f9aa98bba3c] policy::CloudPolicyManager::CreateComponentCloudPolicyService()
[0x7f9aa9518eaf] policy::UserCloudPolicyManager::Connect()
[0x7f9aa951a321] policy::UserPolicySigninService::InitializeUserCloudPolicyManager()
[0x7f9aa951a2af] policy::UserPolicySigninService::OnRefreshTokenAvailable()
[0x7f9aaab639d9] OAuth2TokenService::FireRefreshTokenAvailable()
[0x7f9aa91dbfc9] ProfileOAuth2TokenService::UpdateCredentials()
[0x7f9aa8c9647b] SigninManager::CompletePendingSignin()
[0x7f9aaa2d4b5f] OneClickSigninSyncStarter::ConfirmAndSignin()

BUG=None

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@238381 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 133 insertions, 19 deletions

diff --git a/chrome/browser/policy/cloud/cloud_policy_manager.cc b/chrome/browser/policy/cloud/cloud_policy_manager.cc
index 6856c09..f02b4ab 100644
--- a/chrome/browser/policy/cloud/cloud_policy_manager.cc
+++ b/chrome/browser/policy/cloud/cloud_policy_manager.cc
@@ -132,6 +132,15 @@ void CloudPolicyManager::CreateComponentCloudPolicyService(
 #endif  // !defined(OS_ANDROID) && !defined(OS_IOS)
 }
 
+void CloudPolicyManager::ClearAndDestroyComponentCloudPolicyService() {
+#if !defined(OS_ANDROID) && !defined(OS_IOS)
+  if (component_policy_service_) {
+    component_policy_service_->ClearCache();
+    component_policy_service_.reset();
+  }
+#endif  // !defined(OS_ANDROID) && !defined(OS_IOS)
+}
+
 void CloudPolicyManager::OnRefreshComplete(bool success) {
   waiting_for_policy_refresh_ = false;
   CheckAndPublishPolicy();
diff --git a/chrome/browser/policy/cloud/cloud_policy_manager.h b/chrome/browser/policy/cloud/cloud_policy_manager.h
index 02af206..bc04622 100644
--- a/chrome/browser/policy/cloud/cloud_policy_manager.h
+++ b/chrome/browser/policy/cloud/cloud_policy_manager.h
@@ -77,6 +77,8 @@ class CloudPolicyManager : public ConfigurationPolicyProvider,
       const base::FilePath& policy_cache_path,
       const scoped_refptr<net::URLRequestContextGetter>& request_context);
 
+  void ClearAndDestroyComponentCloudPolicyService();
+
   // Convenience accessors to core() components.
   CloudPolicyClient* client() { return core_.client(); }
   const CloudPolicyClient* client() const { return core_.client(); }
diff --git a/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc b/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc
index aa80432..e143e1d 100644
--- a/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc
+++ b/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc
@@ -4,11 +4,14 @@
 
 #include <string>
 
+#include "base/base64.h"
 #include "base/command_line.h"
+#include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "base/strings/string_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_test_message_listener.h"
@@ -22,6 +25,7 @@
 #include "chrome/browser/policy/test/local_policy_test_server.h"
 #include "chrome/browser/policy/test/policy_test_utils.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "extensions/common/extension.h"
@@ -80,6 +84,15 @@ const char kTestPolicy2[] =
 
 const char kTestPolicy2JSON[] = "{\"Another\":\"turn_it_off\"}";
 
+// Same encoding as ResourceCache does for its keys.
+bool Base64Encode(const std::string& value, std::string* encoded) {
+  if (value.empty() || !base::Base64Encode(value, encoded))
+    return false;
+  ReplaceChars(*encoded, "+", "-", encoded);
+  ReplaceChars(*encoded, "/", "_", encoded);
+  return true;
+}
+
 class ComponentCloudPolicyTest : public ExtensionBrowserTest {
  protected:
   ComponentCloudPolicyTest() {}
@@ -121,6 +134,32 @@ class ComponentCloudPolicyTest : public ExtensionBrowserTest {
     ASSERT_EQ(kTestExtension, extension_->id());
     EXPECT_TRUE(ready_listener.WaitUntilSatisfied());
 
+    // And start with a signed-in user.
+    SignInAndRegister();
+
+    // The extension will receive an update event.
+    EXPECT_TRUE(event_listener_->WaitUntilSatisfied());
+
+    ExtensionBrowserTest::SetUpOnMainThread();
+  }
+
+  scoped_refptr<const extensions::Extension> LoadExtension(
+      const base::FilePath::CharType* path) {
+    base::FilePath full_path;
+    if (!PathService::Get(chrome::DIR_TEST_DATA, &full_path)) {
+      ADD_FAILURE();
+      return NULL;
+    }
+    scoped_refptr<const extensions::Extension> extension(
+        ExtensionBrowserTest::LoadExtension(full_path.Append(path)));
+    if (!extension.get()) {
+      ADD_FAILURE();
+      return NULL;
+    }
+    return extension;
+  }
+
+  void SignInAndRegister() {
     BrowserPolicyConnector* connector =
         g_browser_process->browser_policy_connector();
     connector->ScheduleServiceInitialization(0);
@@ -161,28 +200,16 @@ class ComponentCloudPolicyTest : public ExtensionBrowserTest {
     run_loop.Run();
     Mock::VerifyAndClearExpectations(&observer);
     policy_manager->core()->client()->RemoveObserver(&observer);
-
-    // The extension will receive an update event.
-    EXPECT_TRUE(event_listener_->WaitUntilSatisfied());
-
-    ExtensionBrowserTest::SetUpOnMainThread();
   }
 
-  scoped_refptr<const extensions::Extension> LoadExtension(
-      const base::FilePath::CharType* path) {
-    base::FilePath full_path;
-    if (!PathService::Get(chrome::DIR_TEST_DATA, &full_path)) {
-      ADD_FAILURE();
-      return NULL;
-    }
-    scoped_refptr<const extensions::Extension> extension(
-        ExtensionBrowserTest::LoadExtension(full_path.Append(path)));
-    if (!extension.get()) {
-      ADD_FAILURE();
-      return NULL;
-    }
-    return extension;
+#if !defined(OS_CHROMEOS)
+  void SignOut() {
+    SigninManager* signin_manager =
+        SigninManagerFactory::GetForProfile(browser()->profile());
+    ASSERT_TRUE(signin_manager);
+    signin_manager->SignOut();
   }
+#endif
 
   void RefreshPolicies() {
     ProfilePolicyConnector* profile_connector =
@@ -248,4 +275,61 @@ IN_PROC_BROWSER_TEST_F(ComponentCloudPolicyTest, InstallNewExtension) {
   EXPECT_TRUE(result_listener.WaitUntilSatisfied());
 }
 
+// Signing out on Chrome OS is a different process from signing out on the
+// Desktop platforms. On Chrome OS the session is ended, and the user goes back
+// to the sign-in screen; the Profile data is not affected. On the Desktop the
+// session goes on though, and all the signed-in services are disconnected;
+// in particular, the policy caches are dropped if the user signs out.
+// This test verifies that when the user signs out then any existing component
+// policy caches are dropped, and that it's still possible to sign back in and
+// get policy for components working again.
+#if !defined(OS_CHROMEOS)
+IN_PROC_BROWSER_TEST_F(ComponentCloudPolicyTest, SignOutAndBackIn) {
+  // Read the initial policy.
+  ExtensionTestMessageListener initial_policy_listener(kTestPolicyJSON, true);
+  event_listener_->Reply("get-policy-Name");
+  EXPECT_TRUE(initial_policy_listener.WaitUntilSatisfied());
+
+  // Verify that the policy cache exists.
+  std::string cache_key;
+  ASSERT_TRUE(Base64Encode("extension-policy", &cache_key));
+  std::string cache_subkey;
+  ASSERT_TRUE(Base64Encode(kTestExtension, &cache_subkey));
+  base::FilePath cache_path = browser()->profile()->GetPath()
+      .Append(FILE_PATH_LITERAL("Policy"))
+      .Append(FILE_PATH_LITERAL("Components"))
+      .AppendASCII(cache_key)
+      .AppendASCII(cache_subkey);
+  EXPECT_TRUE(base::PathExists(cache_path));
+
+  // Now sign-out. The policy cache should be removed, and the extension should
+  // get an empty policy update.
+  ExtensionTestMessageListener event_listener("event", true);
+  initial_policy_listener.Reply("idle");
+  SignOut();
+  EXPECT_TRUE(event_listener.WaitUntilSatisfied());
+
+  // The extension got an update event; verify that the policy was empty.
+  ExtensionTestMessageListener signout_policy_listener("{}", true);
+  event_listener.Reply("get-policy-Name");
+  EXPECT_TRUE(signout_policy_listener.WaitUntilSatisfied());
+
+  // Verify that the cache is gone.
+  EXPECT_FALSE(base::PathExists(cache_path));
+
+  // Verify that the policy is fetched again if the user signs back in.
+  ExtensionTestMessageListener event_listener2("event", true);
+  SignInAndRegister();
+  EXPECT_TRUE(event_listener2.WaitUntilSatisfied());
+
+  // The extension got updated policy; verify it.
+  ExtensionTestMessageListener signin_policy_listener(kTestPolicyJSON, true);
+  event_listener2.Reply("get-policy-Name");
+  EXPECT_TRUE(signin_policy_listener.WaitUntilSatisfied());
+
+  // And the cache is back.
+  EXPECT_TRUE(base::PathExists(cache_path));
+}
+#endif
+
 }  // namespace policy
diff --git a/chrome/browser/policy/cloud/component_cloud_policy_service.cc b/chrome/browser/policy/cloud/component_cloud_policy_service.cc
index 3cc210c8..f19b3ff 100644
--- a/chrome/browser/policy/cloud/component_cloud_policy_service.cc
+++ b/chrome/browser/policy/cloud/component_cloud_policy_service.cc
@@ -269,6 +269,15 @@ bool ComponentCloudPolicyService::SupportsDomain(PolicyDomain domain) {
   return ComponentCloudPolicyStore::SupportsDomain(domain);
 }
 
+void ComponentCloudPolicyService::ClearCache() {
+  DCHECK(CalledOnValidThread());
+  // Empty credentials will wipe the cache.
+  backend_task_runner_->PostTask(FROM_HERE,
+                                 base::Bind(&Backend::SetCredentials,
+                                            base::Unretained(backend_.get()),
+                                            std::string(), std::string()));
+}
+
 void ComponentCloudPolicyService::OnSchemaRegistryReady() {
   DCHECK(CalledOnValidThread());
   InitializeIfReady();
diff --git a/chrome/browser/policy/cloud/component_cloud_policy_service.h b/chrome/browser/policy/cloud/component_cloud_policy_service.h
index 75fcef1..95ae7ed 100644
--- a/chrome/browser/policy/cloud/component_cloud_policy_service.h
+++ b/chrome/browser/policy/cloud/component_cloud_policy_service.h
@@ -92,6 +92,9 @@ class ComponentCloudPolicyService : public CloudPolicyClient::Observer,
   // Returns the current policies for components.
   const PolicyBundle& policy() const { return policy_; }
 
+  // Deletes all the cached component policy.
+  void ClearCache();
+
   // SchemaRegistry::Observer implementation:
   virtual void OnSchemaRegistryReady() OVERRIDE;
   virtual void OnSchemaRegistryUpdated(bool has_new_schemas) OVERRIDE;
diff --git a/chrome/browser/policy/cloud/user_cloud_policy_manager.cc b/chrome/browser/policy/cloud/user_cloud_policy_manager.cc
index e74a931..77e3ada 100644
--- a/chrome/browser/policy/cloud/user_cloud_policy_manager.cc
+++ b/chrome/browser/policy/cloud/user_cloud_policy_manager.cc
@@ -66,6 +66,7 @@ void UserCloudPolicyManager::Connect(
                                 policy_prefs::kUserPolicyRefreshRate);
   if (external_data_manager_)
     external_data_manager_->Connect(request_context);
+
   CreateComponentCloudPolicyService(component_policy_cache_path_,
                                     request_context);
 }
@@ -89,6 +90,12 @@ void UserCloudPolicyManager::DisconnectAndRemovePolicy() {
   if (external_data_manager_)
     external_data_manager_->Disconnect();
   core()->Disconnect();
+
+  // store_->Clear() will publish the updated, empty policy. The component
+  // policy service must be cleared before OnStoreLoaded() is issued, so that
+  // component policies are also empty at CheckAndPublishPolicy().
+  ClearAndDestroyComponentCloudPolicyService();
+
   // When the |store_| is cleared, it informs the |external_data_manager_| that
   // all external data references have been removed, causing the
   // |external_data_manager_| to clear its cache as well.