Fix LacrosChrome init data passing.

chromeos::ReadStartupData() can be called in the process at most
once, and later calls will fail.
That caused initialization error.

This CL fixes it by removing the call from PolicyLoaderLacros.
Instead, LacrosChromeServiceImpl is instantiated at earlier
timing (the same timing as D-Bus for ash-chrome), so it should be
available on PolicyLoaderLacros use.

Bug: None
Test: Ran locally on DUT. Made sure no error logs.
Change-Id: I606348c2b1db72d237d05d5a485dfa4f0ae4492b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2626670
Auto-Submit: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Igor <igorcov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843222}

chrome/app/chrome_main_delegate.cc

@@ -527,6 +527,9 @@ void ChromeMainDelegate::PostEarlyInitialization(bool is_running_tests) {
   // a ChromeNetworkDelegate attached that selectively allows cookies again.
   net::URLRequest::SetDefaultCookiePolicyToBlock();
 
+  // On Chrome OS, IPC (D-Bus, Crosapi) is required to create the FeatureList,
+  // which depends on policy from an OS service. So, initialize it at this
+  // timing.
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   // The feature list depends on BrowserPolicyConnectorChromeOS which depends
   // on DBus, so initialize it here. Some D-Bus clients may depend on feature
@@ -534,6 +537,16 @@ void ChromeMainDelegate::PostEarlyInitialization(bool is_running_tests) {
   chromeos::InitializeDBus();
 #endif
 
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
+  // LacrosChromeServiceImpl instance needs the sequence of the main thread,
+  // and needs to be created earlier than incoming Mojo invitation handling.
+  // This also needs ThreadPool sequences to post some tasks internally.
+  // However, the tasks can be suspended until actual start of the ThreadPool
+  // sequences later.
+  lacros_chrome_service_ = std::make_unique<chromeos::LacrosChromeServiceImpl>(
+      std::make_unique<LacrosChromeServiceDelegateImpl>());
+#endif
+
   ChromeFeatureListCreator* chrome_feature_list_creator =
       chrome_content_browser_client_->startup_data()
           ->chrome_feature_list_creator();
@@ -551,13 +564,6 @@ void ChromeMainDelegate::PostEarlyInitialization(bool is_running_tests) {
   chromeos::InitializeFeatureListDependentDBus();
 #endif
 
-#if BUILDFLAG(IS_CHROMEOS_LACROS)
-  // LacrosChromeServiceImpl instance is needs the sequence of the main thread,
-  // and needs to be created earlier than incoming Mojo invitation handling.
-  lacros_chrome_service_ = std::make_unique<chromeos::LacrosChromeServiceImpl>(
-      std::make_unique<LacrosChromeServiceDelegateImpl>());
-#endif
-
 #if defined(OS_ANDROID)
   chrome_content_browser_client_->startup_data()->CreateProfilePrefService();
   net::NetworkChangeNotifier::SetFactory(

components/policy/core/common/policy_loader_lacros.cc

@@ -4,20 +4,18 @@
 
 #include "components/policy/core/common/policy_loader_lacros.h"
 
-#include <stddef.h>
+#include <stdint.h>
 
-#include <algorithm>
-#include <set>
+#include <memory>
 #include <string>
+#include <utility>
+#include <vector>
 
-#include "base/bind.h"
+#include "base/check.h"
 #include "base/logging.h"
-#include "base/stl_util.h"
-#include "base/task/task_traits.h"
-#include "base/task/thread_pool.h"
+#include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "chromeos/lacros/lacros_chrome_service_impl.h"
-#include "chromeos/startup/startup.h"
 #include "components/policy/core/common/cloud/cloud_policy_validator.h"
 #include "components/policy/core/common/policy_bundle.h"
 #include "components/policy/core/common/policy_proto_decoders.h"
@@ -38,37 +36,16 @@ void PolicyLoaderLacros::InitOnBackgroundThread() {
 std::unique_ptr<PolicyBundle> PolicyLoaderLacros::Load() {
   std::unique_ptr<PolicyBundle> bundle = std::make_unique<PolicyBundle>();
 
-  crosapi::mojom::LacrosInitParamsPtr result;
-  const crosapi::mojom::LacrosInitParams* init_params;
   auto* lacros_chrome_service = chromeos::LacrosChromeServiceImpl::Get();
-  if (lacros_chrome_service) {
-    init_params = lacros_chrome_service->init_params();
-  } else {
-    // On the first start of Lacros browser, the lacros_chrome_service is
-    // not initialized yet, so take the data directly from the file. This always
-    // happens on first start after user login, because policy data is loaded
-    // before the service is initialized. We cannot do other way, since there
-    // are other dependencies that create a cycle. The in-memory file is used
-    // to break the cycle. After that, if user reloads the policy the service
-    // is present.
-    // TODO(crbug.com/1114069): This code is duplicated in
-    // LacrosChromeServiceImpl. We could store the data in a static variable
-    // inside LacrosChromeServiceImpl and make a single static function to call.
-    base::Optional<std::string> content = chromeos::ReadStartupData();
-    if (!content) {
-      LOG(ERROR) << "No content in file for init params";
-      return bundle;
-    }
-
-    if (!crosapi::mojom::LacrosInitParams::Deserialize(
-            content->data(), content->size(), &result)) {
-      LOG(ERROR) << "Failed to parse startup data";
-      return bundle;
-    }
-
-    init_params = result.get();
+  if (!lacros_chrome_service) {
+    // LacrosChromeService should be available at this timing in production.
+    // However, in some existing tests, it is not.
+    // TODO(crbug.com/1114069): Set up LacrosChromeServiceImpl in tests.
+    LOG(ERROR) << "No LacrosChromeService is found.";
+    return bundle;
   }
-
+  const crosapi::mojom::LacrosInitParams* init_params =
+      lacros_chrome_service->init_params();
   if (!init_params) {
     LOG(ERROR) << "No init params";
     return bundle;