Revert of Re-enable fetching component policies for login screen apps...

Revert of Re-enable fetching component policies for login screen apps (patchset #3 id:40001 of https://codereview.chromium.org/2603463003/ )

Reason for revert:
I'm very sorry I have to revert this since it's breaking device policy updates.

https://bugs.chromium.org/p/chromium/issues/detail?id=679956#c7

Original issue's description:
> Re-enable fetching component policies for login screen apps
>
> This CL relands enabling fetching of policies for Chrome OS login screen
> apps.
>
> The originally landed code was temporarily disabled due to DMServer
> responding with errors when an unknown policy type was requested. Now
> this can be enabled back, after the DMServer got fixed to at least
> ignore the new policy type (until it starts to actually provide policies
> with this type).
>
> BUG=644304,666720
> TEST=re-enabled browser tests;
>      manual test: sign into an enrolled device, go to chrome://policy
>      and check that the device policies were fetched successfully
>
> Committed: https://crrev.com/5eb4901df37717dd05fd071a807a0ebaf563f0f3
> Cr-Commit-Position: refs/heads/master@{#441015}

TBR=emaxx@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=644304,666720

Review-Url: https://codereview.chromium.org/2621413007
Cr-Commit-Position: refs/heads/master@{#443328}

chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc

@@ -86,19 +86,18 @@ IN_PROC_BROWSER_TEST_F(DeviceCloudPolicyBrowserTest, Initializer) {
 class SigninExtensionsDeviceCloudPolicyBrowserTestBase
     : public DevicePolicyCrosBrowserTest {
  protected:
-  static constexpr const char* kTestExtensionId =
+  constexpr static const char* kTestExtensionId =
       "baogpbmpccplckhhehfipokjaflkmbno";
-  static constexpr const char* kTestExtensionSourceDir =
+  constexpr static const char* kTestExtensionSourceDir =
       "extensions/signin_screen_managed_storage";
-  static constexpr const char* kTestExtensionVersion = "1.0";
-  static constexpr const char* kTestExtensionTestPage = "test.html";
-  static constexpr const char* kFakePolicyUrl =
+  constexpr static const char* kTestExtensionVersion = "1.0";
+  constexpr static const char* kTestExtensionTestPage = "test.html";
+  constexpr static const char* kFakePolicyUrl =
       "http://example.org/test-policy.json";
-  static constexpr const char* kFakePolicy =
+  constexpr static const char* kFakePolicy =
       "{\"string-policy\": {\"Value\": \"value\"}}";
-  static constexpr int kFakePolicyPublicKeyVersion = 1;
-  static constexpr const char* kPolicyProtoCacheKey = "signinextension-policy";
-  static constexpr const char* kPolicyDataCacheKey =
+  constexpr static const char* kPolicyProtoCacheKey = "signinextension-policy";
+  constexpr static const char* kPolicyDataCacheKey =
       "signinextension-policy-data";
 
   SigninExtensionsDeviceCloudPolicyBrowserTestBase() {}
@@ -156,14 +155,13 @@ class SigninExtensionsDeviceCloudPolicyBrowserTestBase
     return extension;
   }
 
-  static enterprise_management::PolicyFetchResponse BuildTestComponentPolicy() {
+  enterprise_management::PolicyFetchResponse BuildTestComponentPolicy() {
     ComponentPolicyBuilder builder;
     MakeTestComponentPolicyBuilder(&builder);
     return builder.policy();
   }
 
-  static enterprise_management::ExternalPolicyData
-  BuildTestComponentPolicyPayload() {
+  enterprise_management::ExternalPolicyData BuildTestComponentPolicyPayload() {
     ComponentPolicyBuilder builder;
     MakeTestComponentPolicyBuilder(&builder);
     return builder.payload();
@@ -172,20 +170,21 @@ class SigninExtensionsDeviceCloudPolicyBrowserTestBase
  private:
   void SetFakeDevicePolicy() {
     device_policy()->policy_data().set_username(PolicyBuilder::kFakeUsername);
-    device_policy()->policy_data().set_public_key_version(
-        kFakePolicyPublicKeyVersion);
+    device_policy()->SetDefaultSigningKey();
     device_policy()->Build();
     session_manager_client()->set_device_policy(device_policy()->GetBlob());
   }
 
-  static void MakeTestComponentPolicyBuilder(ComponentPolicyBuilder* builder) {
+  void MakeTestComponentPolicyBuilder(ComponentPolicyBuilder* builder) {
     builder->policy_data().set_policy_type(
         dm_protocol::kChromeSigninExtensionPolicyType);
-    builder->policy_data().set_username(PolicyBuilder::kFakeUsername);
+    builder->policy_data().set_username(
+        device_policy()->policy_data().username());
     builder->policy_data().set_settings_entity_id(kTestExtensionId);
-    builder->policy_data().set_public_key_version(kFakePolicyPublicKeyVersion);
+    builder->policy_data().set_public_key_version(1);
     builder->payload().set_download_url(kFakePolicyUrl);
     builder->payload().set_secure_hash(crypto::SHA256HashString(kFakePolicy));
+    builder->SetDefaultSigningKey();
     builder->Build();
   }
 
@@ -279,8 +278,10 @@ class SigninExtensionsDeviceCloudPolicyBrowserTest
   std::unique_ptr<base::AutoReset<bool>> signin_policy_provided_disabler_;
 };
 
+// DISABLED: see crbug.com/666720, crbug.com/644304. TODO(emaxx): Enable the
+// test back.
 IN_PROC_BROWSER_TEST_F(SigninExtensionsDeviceCloudPolicyBrowserTest,
-                       InstallAndRunInWindow) {
+                       DISABLED_InstallAndRunInWindow) {
   const extensions::Extension* extension = InstallAndLoadTestExtension();
   ASSERT_TRUE(extension);
   Browser* browser = CreateBrowser(GetSigninProfile());
@@ -299,9 +300,11 @@ IN_PROC_BROWSER_TEST_F(SigninExtensionsDeviceCloudPolicyBrowserTest,
 class PreinstalledSigninExtensionsDeviceCloudPolicyBrowserTest
     : public SigninExtensionsDeviceCloudPolicyBrowserTestBase {
  protected:
-  static constexpr const char* kFakeProfileSourceDir =
+  constexpr static const char* kFakeProfileSourceDir =
       "extensions/profiles/signin_screen_managed_storage";
 
+  std::unique_ptr<base::AutoReset<bool>> signin_policy_provided_disabler_;
+
   bool SetUpUserDataDirectory() override {
     PrefillSigninProfile();
     PrefillComponentPolicyCache();
@@ -315,7 +318,6 @@ class PreinstalledSigninExtensionsDeviceCloudPolicyBrowserTest
         chromeos::GetScopedSigninScreenPolicyProviderDisablerForTesting();
   }
 
- private:
   static void PrefillSigninProfile() {
     base::FilePath profile_source_path;
     EXPECT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &profile_source_path));
@@ -352,12 +354,12 @@ class PreinstalledSigninExtensionsDeviceCloudPolicyBrowserTest
     EXPECT_TRUE(
         cache.Store(kPolicyDataCacheKey, kTestExtensionId, kFakePolicy));
   }
-
-  std::unique_ptr<base::AutoReset<bool>> signin_policy_provided_disabler_;
 };
 
+// DISABLED: see crbug.com/666720, crbug.com/644304. TODO(emaxx): Enable the
+// test back.
 IN_PROC_BROWSER_TEST_F(PreinstalledSigninExtensionsDeviceCloudPolicyBrowserTest,
-                       OfflineStart) {
+                       DISABLED_OfflineStart) {
   const extensions::Extension* extension = GetTestExtension();
   ASSERT_TRUE(extension);
   Browser* browser = CreateBrowser(GetSigninProfile());

chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc

@@ -243,9 +243,7 @@ void DeviceCloudPolicyManagerChromeOS::StartConnection(
   if (ForcedReEnrollmentEnabled())
     client_to_connect->SetStateKeysToUpload(state_keys_broker_->state_keys());
 
-  // Create the component cloud policy service for fetching, caching and
-  // exposing policy for extensions.
-  if (!component_policy_disabled_for_testing_) {
+  if (is_component_policy_enabled_) {
     base::FilePath component_policy_cache_dir;
     CHECK(PathService::Get(chromeos::DIR_SIGNIN_PROFILE_COMPONENT_POLICY,
                            &component_policy_cache_dir));

chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h

@@ -123,12 +123,11 @@ class DeviceCloudPolicyManagerChromeOS : public CloudPolicyManager {
   // associated with this schema registry.
   void SetSigninProfileSchemaRegistry(SchemaRegistry* schema_registry);
 
-  // Sets whether the component cloud policy should be disabled (by skipping
-  // the component cloud policy service creation).
-  void set_component_policy_disabled_for_testing(
-      bool component_policy_disabled_for_testing) {
-    component_policy_disabled_for_testing_ =
-        component_policy_disabled_for_testing;
+  // Sets whether the component cloud policy service should be created.
+  // Defaults to true.
+  void set_is_component_policy_enabled_for_testing(
+      bool is_component_policy_enabled) {
+    is_component_policy_enabled_ = is_component_policy_enabled;
   }
 
  private:
@@ -176,9 +175,12 @@ class DeviceCloudPolicyManagerChromeOS : public CloudPolicyManager {
   std::unique_ptr<ForwardingSchemaRegistry>
       signin_profile_forwarding_schema_registry_;
 
-  // Whether the component cloud policy should be disabled (by skipping the
-  // component cloud policy service creation).
-  bool component_policy_disabled_for_testing_ = false;
+  // Whether the component cloud policy service should be created.
+  // TODO(emaxx): Change the default to true once both the client and the
+  // DMServer are ready for handling policy fetches with the
+  // google/chromeos/signinextension type. See crbug.com/666720,
+  // crbug.com/644304 for reference.
+  bool is_component_policy_enabled_ = false;
 
   base::ObserverList<Observer, true> observers_;
 

chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc

@@ -101,7 +101,7 @@ class TestingDeviceCloudPolicyManagerChromeOS
       : DeviceCloudPolicyManagerChromeOS(std::move(store),
                                          task_runner,
                                          state_keys_broker) {
-    set_component_policy_disabled_for_testing(true);
+    set_is_component_policy_enabled_for_testing(false);
   }
 
   ~TestingDeviceCloudPolicyManagerChromeOS() override {}