Fixed crash in CloudPolicyManager.

chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/policy/cloud/cloud_policy_constants.h"
 #include "chrome/browser/policy/cloud/mock_cloud_policy_client.h"
 #include "chrome/browser/policy/policy_service.h"
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/policy/proto/cloud/chrome_extension_policy.pb.h"
@@ skipping 51 matching lines @@
 
 const char kTestPolicy2[] =
     "{"
     "  \"Another\": {"
     "    \"Value\": \"turn_it_off\""
     "  }"
     "}";
 
 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() {}
   virtual ~ComponentCloudPolicyTest() {}
 
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     ExtensionBrowserTest::SetUpCommandLine(command_line);
 #if defined(OS_CHROMEOS)
     // ExtensionBrowserTest sets the login users to a non-managed value;
     // replace it. This is the default username sent in policy blobs from the
@@ skipping 21 matching lines @@
         << "Pre-existing policies in this machine will make this test fail.";
 
     // Install the initial extension.
     ExtensionTestMessageListener ready_listener("ready", true);
     event_listener_.reset(new ExtensionTestMessageListener("event", true));
     extension_ = LoadExtension(kTestExtensionPath);
     ASSERT_TRUE(extension_.get());
     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);
 
 #if defined(OS_CHROMEOS)
     UserCloudPolicyManagerChromeOS* policy_manager =
         UserCloudPolicyManagerFactoryChromeOS::GetForProfile(
             browser()->profile());
     ASSERT_TRUE(policy_manager);
 #else
@@ skipping 18 matching lines @@
     ASSERT_TRUE(policy_manager->core()->client());
     base::RunLoop run_loop;
     MockCloudPolicyClientObserver observer;
     EXPECT_CALL(observer, OnRegistrationStateChanged(_))
         .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
     policy_manager->core()->client()->AddObserver(&observer);
     policy_manager->core()->client()->SetupRegistration(kDMToken, kDeviceID);
     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 =
         ProfilePolicyConnectorFactory::GetForProfile(browser()->profile());
     PolicyService* policy_service = profile_connector->policy_service();
     base::RunLoop run_loop;
     policy_service->RefreshPolicies(run_loop.QuitClosure());
     run_loop.Run();
   }
 
@@ skipping 45 matching lines @@
   scoped_refptr<const extensions::Extension> extension2 =
       LoadExtension(kTestExtension2Path);
   ASSERT_TRUE(extension2.get());
   ASSERT_EQ(kTestExtension2, extension2->id());
 
   // This extension only sends the 'policy' signal once it receives the policy,
   // and after verifying it has the expected value. Otherwise it sends 'fail'.
   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