Support VariationsRestrictParameter in VariationsService for Chrome OS

chrome/browser/chromeos/policy/variations_service_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 <vector>
+
+#include "base/file_util.h"
+#include "base/files/file_path.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/path_service.h"
+#include "base/stl_util.h"
+#include "chrome/browser/chromeos/cros/cros_in_process_browser_test.h"
+#include "chrome/browser/chromeos/policy/device_policy_builder.h"
+#include "chrome/browser/chromeos/policy/proto/chrome_device_policy.pb.h"
+#include "chrome/browser/metrics/variations/variations_service.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chromeos/dbus/fake_session_manager_client.h"
+#include "chromeos/dbus/mock_dbus_thread_manager.h"
+#include "chromeos/dbus/mock_image_burner_client.h"
+#include "crypto/rsa_private_key.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+using ::testing::AnyNumber;
+using ::testing::Return;
+
+namespace policy {
+
+class DeviceVariationsServicePolicyTest :
+    public chromeos::CrosInProcessBrowserTest {
+ protected:
+  DeviceVariationsServicePolicyTest() {};
+
+  virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
+    chromeos::MockDBusThreadManager* mock_dbus_thread_manager =
+        new chromeos::MockDBusThreadManager;
+
+    EXPECT_CALL(*mock_dbus_thread_manager->mock_image_burner_client(),
+                ResetEventHandlers())
+        .Times(AnyNumber());
+    EXPECT_CALL(*mock_dbus_thread_manager->mock_image_burner_client(),
+                SetEventHandlers(_, _))
+        .Times(AnyNumber());

[Mattias Nissler (ping if slow), 2013/04/19 10:37:00]

I think these are just for silencing gmock? I don't think we should have to put
these into each and every test fixture using MockDBusThreadManager. You might
want to check with satorux@ on how this is supposed to be handled.

[satorux1, 2013/04/19 11:46:20]

We are in the process of removing gmock from chromeos/dbus: crbug.com/223061

A recent patch from keybuk@ https://codereview.chromium.org/13927010/
implemented fakes instead of mocks

[satorux1, 2013/04/19 12:04:13]

Sorry. my previous comment didn't answer your questions. I'm curious why image
burner is involved here. Looks completely unrelated. Maybe we should just remove
the mock image burner class if it's not used meaningfully

[Mathieu, 2013/04/19 17:58:25]

Fakes won't work for this browsertest. Seems like it's meant for unit tests.

[Mathieu, 2013/04/19 17:58:25]

It doesn't seem used, let me know if you want me to remove it from the codebase.
For now I will leave these expectations.

[satorux1, 2013/04/22 01:04:35]

That's fine. haruki@ will be removing unused mocks.

+
+    SetUpSessionManager(mock_dbus_thread_manager);
+
+    chromeos::DBusThreadManager::InitializeForTesting(mock_dbus_thread_manager);
+    CrosInProcessBrowserTest::SetUpInProcessBrowserTestFixture();
+  }
+
+  void SetUpSessionManager(
+    chromeos::MockDBusThreadManager* mock_dbus_thread_manager) {
+    EXPECT_CALL(*mock_dbus_thread_manager, GetSessionManagerClient())
+        .WillRepeatedly(Return(&session_manager_client_));
+
+    // Install the owner key.
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    base::FilePath owner_key_file = temp_dir_.path().AppendASCII("owner.key");
+    std::vector<uint8> owner_key_bits;
+    ASSERT_TRUE(device_policy_.signing_key()->ExportPublicKey(&owner_key_bits));
+    ASSERT_EQ(
+        file_util::WriteFile(
+            owner_key_file,
+            reinterpret_cast<const char*>(vector_as_array(&owner_key_bits)),
+            owner_key_bits.size()),
+        static_cast<int>(owner_key_bits.size()));
+    ASSERT_TRUE(PathService::Override(chrome::FILE_OWNER_KEY, owner_key_file));
+
+    // Setup the device policy DeviceVariationsRestrictParameter.
+    enterprise_management::ChromeDeviceSettingsProto& proto(
+        device_policy_.payload());
+    proto.mutable_variations_parameter()->set_parameter("restricted");
+    RefreshDevicePolicy();
+  }
+
+  void RefreshDevicePolicy() {
+    // Reset the key to its original state.
+    device_policy_.set_signing_key(
+        policy::PolicyBuilder::CreateTestSigningKey());
+    device_policy_.Build();
+    // Trick the device into thinking it's enterprise-enrolled by
+    // removing the local private key.  This will allow it to accept
+    // cloud policy for device owner settings.

[Mattias Nissler (ping if slow), 2013/04/19 10:37:00]

Not sure who wrote that comment, but it's a bit unfortunate. The explanation
should rather be:

The local instance of the private half of the owner key must be dropped as
otherwise the NSS library will tell Chrome that the key is available - which is
incorrect and leads to Chrome behaving as if a local owner were logged in.

[Mathieu, 2013/04/19 17:58:25]

Done.

+    device_policy_.set_signing_key(
+        make_scoped_ptr<crypto::RSAPrivateKey>(NULL));
+    device_policy_.set_new_signing_key(
+        make_scoped_ptr<crypto::RSAPrivateKey>(NULL));
+    session_manager_client_.set_device_policy(device_policy_.GetBlob());
+    session_manager_client_.OnPropertyChangeComplete(true);
+  }
+
+  virtual void TearDownInProcessBrowserTestFixture() OVERRIDE {
+    CrosInProcessBrowserTest::TearDownInProcessBrowserTestFixture();
+    chromeos::DBusThreadManager::Shutdown();
+  }
+
+ private:
+  // Mock out policy loads/stores from/to the device.
+  chromeos::FakeSessionManagerClient session_manager_client_;
+
+  // Stores the device owner key.
+  base::ScopedTempDir temp_dir_;
+
+  // Carries Chrome OS device policies for tests.
+  DevicePolicyBuilder device_policy_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeviceVariationsServicePolicyTest);
+};
+
+IN_PROC_BROWSER_TEST_F(DeviceVariationsServicePolicyTest, VariationsURLValid) {
+  const std::string default_variations_url =
+      chrome_variations::VariationsService::
+          GetDefaultVariationsServerURLForTesting();
+
+  // Device policy has updated the cros settings.
+  EXPECT_EQ(default_variations_url + "?restrict=restricted",
+            chrome_variations::VariationsService::GetVariationsServerURL(
+                g_browser_process->local_state()).spec());
+}
+
+} // namespace policy