Fixed crash in CloudPolicyManager.

chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc

Index: chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc
diff --git a/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc b/chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc
index 7adfe8b879fe523a10bffc71abcb5d658a3e27a2..e99167d661073019a6f45730a230d515c20c6562 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"
@@ -80,6 +83,16 @@ 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) {
+  DCHECK(!value.empty());

[bartfab (slow), 2013/12/02 18:08:46]

Nit 1: #include "base/logging.h"
Nit 2: CHECKs are not really a good tool in tests. Why not return false and let
the ASSERT() wrapped around this call catch the failure?

[Joao da Silva, 2013/12/03 09:02:31]

Right, this was just a lazy copy/paste from resource_cache.cc :-) Done.

+  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);
@@ -159,28 +198,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());

[bartfab (slow), 2013/12/02 18:08:46]

Nit: #include "chrome/browser/ui/browser.h"

[Joao da Silva, 2013/12/03 09:02:31]

Done.

+    ASSERT_TRUE(signin_manager);
+    signin_manager->SignOut();
   }
+#endif
 
   void RefreshPolicies() {
     ProfilePolicyConnector* profile_connector =
@@ -246,4 +273,54 @@ IN_PROC_BROWSER_TEST_F(ComponentCloudPolicyTest, InstallNewExtension) {
   EXPECT_TRUE(result_listener.WaitUntilSatisfied());
 }
 
+// Signing out on Chrome OS doesn't drop the policy caches.

[bartfab (slow), 2013/12/02 18:08:46]

What do you mean by signing out on Chrome OS? You cannot sign out of the browser
on that platform.

[Joao da Silva, 2013/12/03 09:02:31]

Expanded this comment.

+#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