Change the way of passing prefs from ExternalLoader.
ExternalLoaders start with StartLoading and after it loaded
prefs, it used to write prefs to ExternalLoader::prefs_ and then
called LoadFinished. Individual loaders would then read the
prefs through protected |prefs_| member. This is problematic
because if an ExternalLoader is loaded multiple times (this is
possible through CheckForExternalUpdates()), there is a chance
of once load overwriting the prefs of another. This results
in crashes like https://crbug.com/732674.
Instead pass in the prefs as a parameter in LoadFinished.
Multiple inflight load of ExternalLoaders should be fine except
ExternalPrefLoader (other ExternalLoaders load synchronously),
which has couple of sync related observers. Extract priority
sync related logic from ExternalPrefLoader into its own class
so that multiple load is supported. Adds test for the extracted
sync logic in ExternalPrefLoader.
Bug: 732674
Change-Id: Iab2f58c0d8481cf30b1008d694c4664198eb01d8
Reviewed-on: https://chromium-review.googlesource.com/636093
Commit-Queue: Istiaque Ahmed <[email protected]>
Reviewed-by: Xiyuan Xia <[email protected]>
Reviewed-by: Devlin <[email protected]>
Cr-Commit-Position: refs/heads/master@{#500090}diff --git a/chrome/browser/extensions/external_pref_loader.h b/chrome/browser/extensions/external_pref_loader.h
index 82ed4d3a..21778534 100644
--- a/chrome/browser/extensions/external_pref_loader.h
+++ b/chrome/browser/extensions/external_pref_loader.h
@@ -10,28 +10,18 @@
#include "base/compiler_specific.h"
#include "base/macros.h"
-#include "base/scoped_observer.h"
#include "base/values.h"
#include "chrome/browser/extensions/external_loader.h"
-#include "components/browser_sync/profile_sync_service.h"
-#include "components/sync/driver/sync_service_observer.h"
-#include "components/sync_preferences/pref_service_syncable_observer.h"
class Profile;
-namespace sync_preferences {
-class PrefServiceSyncable;
-}
-
namespace extensions {
// A specialization of the ExternalLoader that uses a json file to
// look up which external extensions are registered.
// Instances of this class are expected to be created and destroyed on the UI
// thread and they are expecting public method calls from the UI thread.
-class ExternalPrefLoader : public ExternalLoader,
- public sync_preferences::PrefServiceSyncableObserver,
- public syncer::SyncServiceObserver {
+class ExternalPrefLoader : public ExternalLoader {
public:
enum Options {
NONE = 0,
@@ -71,6 +61,9 @@
private:
friend class base::RefCountedThreadSafe<ExternalLoader>;
friend class ExternalTestingLoader;
+ friend class TestExternalPrefLoader;
+
+ class PrioritySyncReadyWaiter;
// Extracts extension information from a json file serialized by |serializer|.
// |path| is only used for informational purposes (outputted when an error
@@ -80,12 +73,6 @@
base::ValueDeserializer* deserializer,
const base::FilePath& path);
- // sync_preferences::PrefServiceSyncableObserver:
- void OnIsSyncingChanged() override;
-
- // syncer::SyncServiceObserver
- void OnStateChanged(syncer::SyncService* sync) override;
-
// If priority sync ready posts LoadOnFileThread and return true.
bool PostLoadIfPrioritySyncReady();
@@ -95,7 +82,8 @@
// Actually searches for and loads candidate standalone extension preference
// files in the path corresponding to |base_path_id|.
// Must be called on the file thread.
- void LoadOnFileThread();
+ // Note: Overridden in tests.
+ virtual void LoadOnFileThread();
// Extracts the information contained in an external_extension.json file
// regarding which extensions to install. |prefs| will be modified to
@@ -110,6 +98,8 @@
// Must be called from the File thread.
void ReadStandaloneExtensionPrefFiles(base::DictionaryValue* prefs);
+ void OnPrioritySyncReady(PrioritySyncReadyWaiter* waiter);
+
// The path (coresponding to |base_path_id_| containing the json files
// describing which extensions to load.
base::FilePath base_path_;
@@ -118,10 +108,7 @@
// Needed for waiting for waiting priority sync.
Profile* profile_;
- // Used for registering observer for sync_preferences::PrefServiceSyncable.
- ScopedObserver<sync_preferences::PrefServiceSyncable,
- sync_preferences::PrefServiceSyncableObserver>
- syncable_pref_observer_;
+ std::vector<std::unique_ptr<PrioritySyncReadyWaiter>> pending_waiter_list_;
DISALLOW_COPY_AND_ASSIGN(ExternalPrefLoader);
};
