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 <[email protected]>
Reviewed-by: Hidehiko Abe <[email protected]>
Cr-Commit-Position: refs/heads/master@{#549204}diff --git a/components/arc/arc_prefs.cc b/components/arc/arc_prefs.cc
index 47cbeb8c..d60f5d5 100644
--- a/components/arc/arc_prefs.cc
+++ b/components/arc/arc_prefs.cc
@@ -29,6 +29,14 @@
// 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 @@
// 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);
diff --git a/components/arc/arc_prefs.h b/components/arc/arc_prefs.h
index 46b3bfd..99fd759c 100644
--- a/components/arc/arc_prefs.h
+++ b/components/arc/arc_prefs.h
@@ -18,6 +18,7 @@
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[];
diff --git a/components/arc/test/fake_backup_settings_instance.cc b/components/arc/test/fake_backup_settings_instance.cc
index 8465281..9a3e908 100644
--- a/components/arc/test/fake_backup_settings_instance.cc
+++ b/components/arc/test/fake_backup_settings_instance.cc
@@ -11,11 +11,11 @@
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;
}
diff --git a/components/arc/test/fake_backup_settings_instance.h b/components/arc/test/fake_backup_settings_instance.h
index 7f4212dc..5ad0d194 100644
--- a/components/arc/test/fake_backup_settings_instance.h
+++ b/components/arc/test/fake_backup_settings_instance.h
@@ -20,12 +20,12 @@
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;
diff --git a/components/arc/test/fake_intent_helper_instance.cc b/components/arc/test/fake_intent_helper_instance.cc
index 93954730..4166b32 100644
--- a/components/arc/test/fake_intent_helper_instance.cc
+++ b/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 @@
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
diff --git a/components/arc/test/fake_intent_helper_instance.h b/components/arc/test/fake_intent_helper_instance.h
index e70cca65..e97d0a2e 100644
--- a/components/arc/test/fake_intent_helper_instance.h
+++ b/components/arc/test/fake_intent_helper_instance.h
@@ -55,6 +55,9 @@
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(
