arc: Fix race condition applying location and B&R settings.

This fix race condition when settings are driven by mojo instance
creation for intent helper and onArcInitialStart is called due the
communication via auth mojom. That leads to case when onArcInitialStart
is not called for arc settings and these settings are not applied on
Android side.

Bug: 822459
Test: Unit tests added. Manually on device. Simulate race condition,
observer that all consumers of onArcInitialStart are called. Only
settings component has such race. Other consumers are created on
session start and do not have this race.

Change-Id: Iab0a5fe53d55c3e84fbe532cf7c8d5b720e67bcb
Reviewed-on: https://chromium-review.googlesource.com/967069
Commit-Queue: Yury Khmel <khmel@google.com>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549204}

chrome/browser/chromeos/BUILD.gn

@@ -1863,6 +1863,7 @@ source_set("unit_tests") {
     "arc/fileapi/file_stream_forwarder_unittest.cc",
     "arc/intent_helper/arc_external_protocol_dialog_unittest.cc",
     "arc/intent_helper/arc_navigation_throttle_unittest.cc",
+    "arc/intent_helper/arc_settings_service_unittest.cc",
     "arc/intent_helper/open_with_menu_unittest.cc",
     "arc/kiosk/arc_kiosk_bridge_unittest.cc",
     "arc/notification/arc_provision_notification_service_unittest.cc",

chrome/browser/chromeos/arc/arc_session_manager.h

@@ -111,7 +111,8 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
     // Called to notify that ARC begins to start.
     virtual void OnArcStarted() {}
 
-    // Called to notify that ARC has been initialized successfully.
+    // Called to notify that ARC has been successfully provisioned for the first
+    // time after OptIn.
     virtual void OnArcInitialStart() {}
 
     // Called when ARC session is stopped, and is not being restarted

chrome/browser/chromeos/arc/arc_session_manager_unittest.cc

@@ -64,6 +64,31 @@ namespace arc {
 
 namespace {
 
+class ArcInitialStartHandler : public ArcSessionManager::Observer {
+ public:
+  explicit ArcInitialStartHandler(ArcSessionManager* session_manager)
+      : session_manager_(session_manager) {
+    session_manager->AddObserver(this);
+  }
+
+  ~ArcInitialStartHandler() override { session_manager_->RemoveObserver(this); }
+
+  // ArcSessionManager::Observer:
+  void OnArcInitialStart() override {
+    DCHECK(!was_called_);
+    was_called_ = true;
+  }
+
+  bool was_called() const { return was_called_; }
+
+ private:
+  bool was_called_ = false;
+
+  ArcSessionManager* const session_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(ArcInitialStartHandler);
+};
+
 class ArcSessionManagerInLoginScreenTest : public testing::Test {
  public:
   ArcSessionManagerInLoginScreenTest()
@@ -275,6 +300,52 @@ TEST_F(ArcSessionManagerTest, BaseWorkflow) {
   arc_session_manager()->Shutdown();
 }
 
+// Tests that OnArcInitialStart is called  after the successful ARC provisioning
+// on the first start after OptIn.
+TEST_F(ArcSessionManagerTest, ArcInitialStartFirstProvisioning) {
+  arc_session_manager()->SetProfile(profile());
+  arc_session_manager()->Initialize();
+
+  ArcInitialStartHandler start_handler(arc_session_manager());
+  EXPECT_FALSE(start_handler.was_called());
+
+  arc_session_manager()->RequestEnable();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_FALSE(start_handler.was_called());
+
+  arc_session_manager()->OnTermsOfServiceNegotiatedForTesting(true);
+  arc_session_manager()->StartArcForTesting();
+
+  EXPECT_FALSE(start_handler.was_called());
+
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  EXPECT_TRUE(start_handler.was_called());
+
+  arc_session_manager()->Shutdown();
+}
+
+// Tests that OnArcInitialStart is not called after the successful ARC
+// provisioning on the second and next starts after OptIn.
+TEST_F(ArcSessionManagerTest, ArcInitialStartNextProvisioning) {
+  // Set up the situation that provisioning is successfully done in the
+  // previous session. In this case initial start callback is not called.
+  PrefService* const prefs = profile()->GetPrefs();
+  prefs->SetBoolean(prefs::kArcTermsAccepted, true);
+  prefs->SetBoolean(prefs::kArcSignedIn, true);
+
+  arc_session_manager()->SetProfile(profile());
+  arc_session_manager()->Initialize();
+
+  ArcInitialStartHandler start_handler(arc_session_manager());
+
+  arc_session_manager()->RequestEnable();
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  EXPECT_FALSE(start_handler.was_called());
+
+  arc_session_manager()->Shutdown();
+}
+
 TEST_F(ArcSessionManagerTest, IncompatibleFileSystemBlocksTermsOfService) {
   SetArcBlockedDueToIncompatibleFileSystemForTesting(true);
 

chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc

@@ -14,7 +14,7 @@
 #include "base/memory/singleton.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/chromeos/arc/arc_session_manager.h"
+#include "chrome/browser/chromeos/arc/arc_util.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/system/timezone_resolver_manager.h"
 #include "chrome/browser/profiles/profile.h"
@@ -107,11 +107,10 @@ class ArcSettingsServiceFactory
 // about and sends the new values to Android to keep the state in sync.
 class ArcSettingsServiceImpl
     : public chromeos::system::TimezoneSettings::Observer,
-      public ArcSessionManager::Observer,
       public ConnectionObserver<mojom::AppInstance>,
       public chromeos::NetworkStateHandlerObserver {
  public:
-  ArcSettingsServiceImpl(content::BrowserContext* context,
+  ArcSettingsServiceImpl(Profile* profile,
                          ArcBridgeService* arc_bridge_service);
   ~ArcSettingsServiceImpl() override;
 
@@ -122,12 +121,13 @@ class ArcSettingsServiceImpl
   // TimezoneSettings::Observer:
   void TimezoneChanged(const icu::TimeZone& timezone) override;
 
-  // ArcSessionManager::Observer:
-  void OnArcInitialStart() override;
-
   // NetworkStateHandlerObserver:
   void DefaultNetworkChanged(const chromeos::NetworkState* network) override;
 
+  // Retrieves Chrome's state for the settings that need to be synced on the
+  // initial Android boot and send it to Android. Called by ArcSettingsService.
+  void SyncInitialSettings() const;
+
  private:
   PrefService* GetPrefs() const { return profile_->GetPrefs(); }
 
@@ -140,9 +140,6 @@ class ArcSettingsServiceImpl
   // Stops listening for Chrome settings changes.
   void StopObservingSettingsChanges();
 
-  // Retrieves Chrome's state for the settings that need to be synced on the
-  // initial Android boot and send it to Android.
-  void SyncInitialSettings() const;
   // Retrieves Chrome's state for the settings that need to be synced on each
   // Android boot and send it to Android.
   void SyncBootTimeSettings() const;
@@ -209,23 +206,15 @@ class ArcSettingsServiceImpl
   std::unique_ptr<ChromeZoomLevelPrefs::DefaultZoomLevelSubscription>
       default_zoom_level_subscription_;
 
-  base::WeakPtrFactory<ArcSettingsServiceImpl> weak_factory_;
-
   DISALLOW_COPY_AND_ASSIGN(ArcSettingsServiceImpl);
 };
 
 ArcSettingsServiceImpl::ArcSettingsServiceImpl(
-    content::BrowserContext* context,
+    Profile* profile,
     ArcBridgeService* arc_bridge_service)
-    : profile_(Profile::FromBrowserContext(context)),
-      arc_bridge_service_(arc_bridge_service),
-      weak_factory_(this) {
-  DCHECK(profile_);
-
+    : profile_(profile), arc_bridge_service_(arc_bridge_service) {
   StartObservingSettingsChanges();
   SyncBootTimeSettings();
-  DCHECK(ArcSessionManager::Get());
-  ArcSessionManager::Get()->AddObserver(this);
 
   // Note: if App connection is already established, OnConnectionReady()
   // is synchronously called, so that initial sync is done in the method.
@@ -236,10 +225,6 @@ ArcSettingsServiceImpl::~ArcSettingsServiceImpl() {
   StopObservingSettingsChanges();
 
   arc_bridge_service_->app()->RemoveObserver(this);
-
-  ArcSessionManager* arc_session_manager = ArcSessionManager::Get();
-  if (arc_session_manager)
-    arc_session_manager->RemoveObserver(this);
 }
 
 void ArcSettingsServiceImpl::OnPrefChanged(const std::string& pref_name) const {
@@ -290,10 +275,6 @@ void ArcSettingsServiceImpl::TimezoneChanged(const icu::TimeZone& timezone) {
   SyncTimeZone();
 }
 
-void ArcSettingsServiceImpl::OnArcInitialStart() {
-  SyncInitialSettings();
-}
-
 void ArcSettingsServiceImpl::DefaultNetworkChanged(
     const chromeos::NetworkState* network) {
   // kProxy pref has more priority than the default network update.
@@ -704,21 +685,55 @@ ArcSettingsService* ArcSettingsService::GetForBrowserContext(
 
 ArcSettingsService::ArcSettingsService(content::BrowserContext* context,
                                        ArcBridgeService* bridge_service)
-    : context_(context), arc_bridge_service_(bridge_service) {
+    : profile_(Profile::FromBrowserContext(context)),
+      arc_bridge_service_(bridge_service) {
   arc_bridge_service_->intent_helper()->AddObserver(this);
+  ArcSessionManager::Get()->AddObserver(this);
+
+  if (!IsArcPlayStoreEnabledForProfile(profile_))
+    SetInitialSettingsPending(false);
 }
 
 ArcSettingsService::~ArcSettingsService() {
+  ArcSessionManager::Get()->RemoveObserver(this);
   arc_bridge_service_->intent_helper()->RemoveObserver(this);
 }
 
 void ArcSettingsService::OnConnectionReady() {
   impl_ =
-      std::make_unique<ArcSettingsServiceImpl>(context_, arc_bridge_service_);
+      std::make_unique<ArcSettingsServiceImpl>(profile_, arc_bridge_service_);
+  if (!IsInitialSettingsPending())
+    return;
+  impl_->SyncInitialSettings();
+  SetInitialSettingsPending(false);
 }
 
 void ArcSettingsService::OnConnectionClosed() {
   impl_.reset();
 }
 
+void ArcSettingsService::OnArcPlayStoreEnabledChanged(bool enabled) {
+  if (!enabled)
+    SetInitialSettingsPending(false);
+}
+
+void ArcSettingsService::OnArcInitialStart() {
+  DCHECK(!IsInitialSettingsPending());
+
+  if (!impl_) {
+    SetInitialSettingsPending(true);
+    return;
+  }
+
+  impl_->SyncInitialSettings();
+}
+
+void ArcSettingsService::SetInitialSettingsPending(bool pending) {
+  profile_->GetPrefs()->SetBoolean(prefs::kArcInitialSettingsPending, pending);
+}
+
+bool ArcSettingsService::IsInitialSettingsPending() const {
+  return profile_->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending);
+}
+
 }  // namespace arc

chrome/browser/chromeos/arc/intent_helper/arc_settings_service.h

@@ -8,10 +8,13 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "chrome/browser/chromeos/arc/arc_session_manager.h"
 #include "components/arc/common/intent_helper.mojom.h"
 #include "components/arc/connection_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
 
+class Profile;
+
 namespace content {
 class BrowserContext;
 }  // namespace content
@@ -23,7 +26,8 @@ class ArcSettingsServiceImpl;
 
 class ArcSettingsService
     : public KeyedService,
-      public ConnectionObserver<mojom::IntentHelperInstance> {
+      public ConnectionObserver<mojom::IntentHelperInstance>,
+      public ArcSessionManager::Observer {
  public:
   // Returns singleton instance for the given BrowserContext,
   // or nullptr if the browser |context| is not allowed to use ARC.
@@ -38,8 +42,15 @@ class ArcSettingsService
   void OnConnectionReady() override;
   void OnConnectionClosed() override;
 
+  // ArcSessionManager::Observer:
+  void OnArcPlayStoreEnabledChanged(bool enabled) override;
+  void OnArcInitialStart() override;
+
  private:
-  content::BrowserContext* const context_;
+  void SetInitialSettingsPending(bool pending);
+  bool IsInitialSettingsPending() const;
+
+  Profile* const profile_;
   ArcBridgeService* const arc_bridge_service_;  // Owned by ArcServiceManager.
   std::unique_ptr<ArcSettingsServiceImpl> impl_;
 

chrome/browser/chromeos/arc/intent_helper/arc_settings_service_browsertest.cc

@@ -355,7 +355,7 @@ IN_PROC_BROWSER_TEST_F(ArcSettingsServiceTest, BackupRestorePolicyTest) {
   // not reflect the pref as it is not dynamically applied.
   EXPECT_FALSE(prefs->GetBoolean(prefs::kArcBackupRestoreEnabled));
   EXPECT_TRUE(prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled));
-  EXPECT_FALSE(fake_backup_settings_instance_->set_backup_enabled_called());
+  EXPECT_EQ(0, fake_backup_settings_instance_->set_backup_enabled_count());
   EXPECT_FALSE(fake_backup_settings_instance_->enabled());
   EXPECT_FALSE(fake_backup_settings_instance_->managed());
 
@@ -372,7 +372,7 @@ IN_PROC_BROWSER_TEST_F(ArcSettingsServiceTest, BackupRestorePolicyTest) {
   // not reflect the pref as it is not dynamically applied.
   EXPECT_TRUE(prefs->GetBoolean(prefs::kArcBackupRestoreEnabled));
   EXPECT_TRUE(prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled));
-  EXPECT_FALSE(fake_backup_settings_instance_->set_backup_enabled_called());
+  EXPECT_EQ(0, fake_backup_settings_instance_->set_backup_enabled_count());
   EXPECT_FALSE(fake_backup_settings_instance_->enabled());
   EXPECT_FALSE(fake_backup_settings_instance_->managed());
 
@@ -386,7 +386,7 @@ IN_PROC_BROWSER_TEST_F(ArcSettingsServiceTest, BackupRestorePolicyTest) {
   // the pref as it is not dynamically applied.
   EXPECT_TRUE(prefs->GetBoolean(prefs::kArcBackupRestoreEnabled));
   EXPECT_FALSE(prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled));
-  EXPECT_FALSE(fake_backup_settings_instance_->set_backup_enabled_called());
+  EXPECT_EQ(0, fake_backup_settings_instance_->set_backup_enabled_count());
   EXPECT_FALSE(fake_backup_settings_instance_->enabled());
   EXPECT_FALSE(fake_backup_settings_instance_->managed());
 }

chrome/browser/chromeos/arc/intent_helper/arc_settings_service_unittest.cc

+// Copyright 2018 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 "chrome/browser/chromeos/arc/intent_helper/arc_settings_service.h"
+
+#include <memory>
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "chrome/browser/chromeos/arc/arc_optin_uma.h"
+#include "chrome/browser/chromeos/arc/arc_session_manager.h"
+#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
+#include "chrome/test/base/browser_with_test_window_test.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/network/network_handler.h"
+#include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_prefs.h"
+#include "components/arc/arc_service_manager.h"
+#include "components/arc/arc_util.h"
+#include "components/arc/test/connection_holder_util.h"
+#include "components/arc/test/fake_arc_session.h"
+#include "components/arc/test/fake_backup_settings_instance.h"
+#include "components/arc/test/fake_intent_helper_instance.h"
+#include "components/prefs/pref_service.h"
+#include "components/user_manager/scoped_user_manager.h"
+#include "components/user_manager/user_manager.h"
+
+namespace arc {
+
+namespace {
+
+constexpr char kActionLocaionEnabled[] =
+    "org.chromium.arc.intent_helper.SET_LOCATION_SERVICE_ENABLED";
+
+class ArcSettingsServiceTest : public BrowserWithTestWindowTest {
+ public:
+  ArcSettingsServiceTest()
+      : user_manager_enabler_(
+            std::make_unique<chromeos::FakeChromeUserManager>()) {}
+  ~ArcSettingsServiceTest() override = default;
+
+  // BrowserWithTestWindowTest:
+  void SetUp() override {
+    SetArcAvailableCommandLineForTesting(
+        base::CommandLine::ForCurrentProcess());
+    ArcSessionManager::DisableUIForTesting();
+    chromeos::DBusThreadManager::Initialize();
+    chromeos::NetworkHandler::Initialize();
+
+    arc_service_manager_ = std::make_unique<ArcServiceManager>();
+    arc_session_manager_ =
+        std::make_unique<ArcSessionManager>(std::make_unique<ArcSessionRunner>(
+            base::BindRepeating(FakeArcSession::Create)));
+
+    BrowserWithTestWindowTest::SetUp();
+    arc_service_manager_->set_browser_context(profile());
+
+    const AccountId account_id(AccountId::FromUserEmailGaiaId(
+        profile()->GetProfileUserName(), "1234567890"));
+    user_manager()->AddUser(account_id);
+    user_manager()->LoginUser(account_id);
+
+    arc_session_manager()->SetProfile(profile());
+    arc_session_manager()->Initialize();
+
+    arc_intent_helper_bridge_ = std::make_unique<ArcIntentHelperBridge>(
+        profile(), arc_bridge_service());
+    ArcSettingsService* arc_settings_service =
+        ArcSettingsService::GetForBrowserContext(profile());
+    DCHECK(arc_settings_service);
+
+    // These prefs are set in negotiator.
+    profile()->GetPrefs()->SetBoolean(prefs::kArcLocationServiceEnabled, true);
+    profile()->GetPrefs()->SetBoolean(prefs::kArcBackupRestoreEnabled, true);
+  }
+
+  void TearDown() override {
+    arc_bridge_service()->intent_helper()->CloseInstance(
+        &intent_helper_instance_);
+    arc_bridge_service()->backup_settings()->CloseInstance(
+        &backup_settings_instance_);
+    arc_intent_helper_bridge_.reset();
+    arc_session_manager()->Shutdown();
+
+    arc_service_manager_->set_browser_context(nullptr);
+    BrowserWithTestWindowTest::TearDown();
+
+    arc_session_manager_.reset();
+    arc_service_manager_.reset();
+
+    chromeos::NetworkHandler::Shutdown();
+    chromeos::DBusThreadManager::Shutdown();
+  }
+
+  void SetInstances() {
+    arc_bridge_service()->backup_settings()->SetInstance(
+        &backup_settings_instance_);
+    WaitForInstanceReady(arc_bridge_service()->backup_settings());
+
+    arc_bridge_service()->intent_helper()->SetInstance(
+        &intent_helper_instance_);
+    WaitForInstanceReady(arc_bridge_service()->intent_helper());
+  }
+
+  chromeos::FakeChromeUserManager* user_manager() {
+    return static_cast<chromeos::FakeChromeUserManager*>(
+        user_manager::UserManager::Get());
+  }
+  ArcBridgeService* arc_bridge_service() {
+    return arc_service_manager_->arc_bridge_service();
+  }
+  ArcSessionManager* arc_session_manager() {
+    return arc_session_manager_.get();
+  }
+  FakeIntentHelperInstance* intent_helper_instance() {
+    return &intent_helper_instance_;
+  }
+  FakeBackupSettingsInstance* backup_settings_instance() {
+    return &backup_settings_instance_;
+  }
+
+ private:
+  user_manager::ScopedUserManager user_manager_enabler_;
+  std::unique_ptr<ArcIntentHelperBridge> arc_intent_helper_bridge_;
+  std::unique_ptr<ArcSessionManager> arc_session_manager_;
+  std::unique_ptr<ArcServiceManager> arc_service_manager_;
+  FakeIntentHelperInstance intent_helper_instance_;
+  FakeBackupSettingsInstance backup_settings_instance_;
+
+  DISALLOW_COPY_AND_ASSIGN(ArcSettingsServiceTest);
+};
+
+}  // namespace
+
+// Initial settings applied in case intent helper instance is set after
+// provisioning.
+TEST_F(ArcSettingsServiceTest,
+       InitialSettingsAppliedForInstanceAfterProvisioning) {
+  arc_session_manager()->RequestEnable();
+  arc_session_manager()->OnTermsOfServiceNegotiatedForTesting(true);
+  arc_session_manager()->StartArcForTesting();
+
+  EXPECT_FALSE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending));
+
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+
+  EXPECT_TRUE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending));
+  EXPECT_EQ(0, backup_settings_instance()->set_backup_enabled_count());
+  EXPECT_TRUE(intent_helper_instance()
+                  ->GetBroadcastsForAction(kActionLocaionEnabled)
+                  .empty());
+
+  SetInstances();
+  EXPECT_FALSE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending));
+
+  EXPECT_EQ(1, backup_settings_instance()->set_backup_enabled_count());
+  EXPECT_EQ(1U, intent_helper_instance()
+                    ->GetBroadcastsForAction(kActionLocaionEnabled)
+                    .size());
+}
+
+// Initial settings applied in case intent helper instance is set before
+// provisioning.
+TEST_F(ArcSettingsServiceTest,
+       InitialSettingsAppliedForInstanceBeforeProvisioning) {
+  arc_session_manager()->RequestEnable();
+  arc_session_manager()->OnTermsOfServiceNegotiatedForTesting(true);
+  arc_session_manager()->StartArcForTesting();
+
+  SetInstances();
+  EXPECT_FALSE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending));
+
+  EXPECT_EQ(0, backup_settings_instance()->set_backup_enabled_count());
+  EXPECT_TRUE(intent_helper_instance()
+                  ->GetBroadcastsForAction(kActionLocaionEnabled)
+                  .empty());
+
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+
+  EXPECT_FALSE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending));
+  EXPECT_EQ(1, backup_settings_instance()->set_backup_enabled_count());
+  EXPECT_EQ(1U, intent_helper_instance()
+                    ->GetBroadcastsForAction(kActionLocaionEnabled)
+                    .size());
+}
+
+// Initial settings are applied in case intent helper instance was not set in
+// the first session when OptIn happened but for set in the next session.
+TEST_F(ArcSettingsServiceTest, InitialSettingsPendingAppliedNextSession) {
+  profile()->GetPrefs()->SetBoolean(prefs::kArcInitialSettingsPending, true);
+  profile()->GetPrefs()->SetBoolean(prefs::kArcTermsAccepted, true);
+  profile()->GetPrefs()->SetBoolean(prefs::kArcSignedIn, true);
+
+  arc_session_manager()->RequestEnable();
+  SetInstances();
+
+  EXPECT_EQ(1, backup_settings_instance()->set_backup_enabled_count());
+  EXPECT_EQ(1U, intent_helper_instance()
+                    ->GetBroadcastsForAction(kActionLocaionEnabled)
+                    .size());
+  EXPECT_FALSE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending));
+}
+
+// Initial settings are not applied in case intent helper instance is set in the
+// next sessions and we don't have pending request..
+TEST_F(ArcSettingsServiceTest, InitialSettingsNotAppliedNextSession) {
+  profile()->GetPrefs()->SetBoolean(prefs::kArcInitialSettingsPending, false);
+  profile()->GetPrefs()->SetBoolean(prefs::kArcTermsAccepted, true);
+  profile()->GetPrefs()->SetBoolean(prefs::kArcSignedIn, true);
+
+  arc_session_manager()->RequestEnable();
+  SetInstances();
+
+  EXPECT_EQ(0, backup_settings_instance()->set_backup_enabled_count());
+  EXPECT_EQ(0U, intent_helper_instance()
+                    ->GetBroadcastsForAction(kActionLocaionEnabled)
+                    .size());
+  EXPECT_FALSE(
+      profile()->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending));
+}
+
+}  // namespace arc

components/arc/arc_prefs.cc

@@ -29,6 +29,14 @@ const char kArcDataRemoveRequested[] = "arc.data.remove_requested";
 // utility methods (IsArcPlayStoreEnabledForProfile() and
 // SetArcPlayStoreEnabledForProfile()) in chrome/browser/chromeos/arc/arc_util.
 const char kArcEnabled[] = "arc.enabled";
+// A preference that indicates that initial settings need to be applied. Initial
+// settings are applied only once per new OptIn once mojo settings instance is
+// ready. Each OptOut resets this preference. Note, its sense is close to
+// |kArcSignedIn|, however due the asynchronous nature of initializing mojo
+// components, timing of triggering |kArcSignedIn| and
+// |kArcInitialSettingsPending| can be different and
+// |kArcInitialSettingsPending| may even be handled in the next user session.
+const char kArcInitialSettingsPending[] = "arc.initial.settings.pending";
 // A preference that indicated whether Android reported it's compliance status
 // with provided policies. This is used only as a signal to start Android kiosk.
 const char kArcPolicyComplianceReported[] = "arc.policy_compliance_reported";
@@ -94,6 +102,7 @@ void RegisterProfilePrefs(PrefRegistrySimple* registry) {
   // Sorted in lexicographical order.
   registry->RegisterBooleanPref(kArcDataRemoveRequested, false);
   registry->RegisterBooleanPref(kArcEnabled, false);
+  registry->RegisterBooleanPref(kArcInitialSettingsPending, false);
   registry->RegisterBooleanPref(kArcPaiStarted, false);
   registry->RegisterBooleanPref(kArcPolicyComplianceReported, false);
   registry->RegisterBooleanPref(kArcSignedIn, false);

components/arc/arc_prefs.h

@@ -18,6 +18,7 @@ ARC_EXPORT extern const char kArcApps[];
 ARC_EXPORT extern const char kArcBackupRestoreEnabled[];
 ARC_EXPORT extern const char kArcDataRemoveRequested[];
 ARC_EXPORT extern const char kArcEnabled[];
+ARC_EXPORT extern const char kArcInitialSettingsPending[];
 ARC_EXPORT extern const char kArcPolicyComplianceReported[];
 ARC_EXPORT extern const char kArcTermsAccepted[];
 ARC_EXPORT extern const char kArcTermsShownInOobe[];

components/arc/test/fake_backup_settings_instance.cc

@@ -11,11 +11,11 @@ FakeBackupSettingsInstance::FakeBackupSettingsInstance() = default;
 FakeBackupSettingsInstance::~FakeBackupSettingsInstance() = default;
 
 void FakeBackupSettingsInstance::ClearCallHistory() {
-  set_backup_enabled_called_ = false;
+  set_backup_enabled_count_ = 0;
 }
 
 void FakeBackupSettingsInstance::SetBackupEnabled(bool enabled, bool managed) {
-  set_backup_enabled_called_ = true;
+  ++set_backup_enabled_count_;
   enabled_ = enabled;
   managed_ = managed;
 }

components/arc/test/fake_backup_settings_instance.h

@@ -20,12 +20,12 @@ class FakeBackupSettingsInstance : public mojom::BackupSettingsInstance {
 
   void ClearCallHistory();
 
-  bool set_backup_enabled_called() const { return set_backup_enabled_called_; }
+  int set_backup_enabled_count() const { return set_backup_enabled_count_; }
   bool enabled() const { return enabled_; }
   bool managed() const { return managed_; }
 
  private:
-  bool set_backup_enabled_called_ = false;
+  int set_backup_enabled_count_ = 0;
   bool enabled_ = false;
   bool managed_ = false;
 

components/arc/test/fake_intent_helper_instance.cc

@@ -4,6 +4,8 @@
 
 #include "components/arc/test/fake_intent_helper_instance.h"
 
+#include <algorithm>
+#include <iterator>
 #include <utility>
 
 #include "base/bind.h"
@@ -121,4 +123,14 @@ void FakeIntentHelperInstance::SendBroadcast(const std::string& action,
   broadcasts_.emplace_back(action, package_name, cls, extras);
 }
 
+std::vector<FakeIntentHelperInstance::Broadcast>
+FakeIntentHelperInstance::GetBroadcastsForAction(
+    const std::string& action) const {
+  std::vector<Broadcast> result;
+  std::copy_if(broadcasts_.begin(), broadcasts_.end(),
+               std::back_inserter(result),
+               [action](const Broadcast& b) { return b.action == action; });
+  return result;
+}
+
 }  // namespace arc

components/arc/test/fake_intent_helper_instance.h

@@ -55,6 +55,9 @@ class FakeIntentHelperInstance : public mojom::IntentHelperInstance {
     return handled_intents_;
   }
 
+  std::vector<Broadcast> GetBroadcastsForAction(
+      const std::string& action) const;
+
   // Sets a list of intent handlers to be returned in response to
   // RequestIntentHandlerList() calls with intents containing |action|.
   void SetIntentHandlers(