Refactoring the WeakPtrFactory usage, Member variables should appear before...

Refactoring the WeakPtrFactory usage, Member variables should appear before the WeakPtrFactory, to ensure that any WeakPtrs to Controller are invalidated before its members variable's destructors are executed, rendering them invalid.

BUG=303818

Review URL: https://codereview.chromium.org/554233002

Cr-Commit-Position: refs/heads/master@{#294163}

components/autofill/core/browser/webdata/autofill_webdata_service.cc

@@ -33,8 +33,8 @@ AutofillWebDataService::AutofillWebDataService(
     : WebDataServiceBase(wdbs, callback, ui_thread),
       ui_thread_(ui_thread),
       db_thread_(db_thread),
-      weak_ptr_factory_(this),
-      autofill_backend_(NULL) {
+      autofill_backend_(NULL),
+      weak_ptr_factory_(this) {
 
   base::Closure on_changed_callback = Bind(
       &AutofillWebDataService::NotifyAutofillMultipleChangedOnUIThread,
@@ -51,11 +51,11 @@ AutofillWebDataService::AutofillWebDataService(
           ui_thread),
       ui_thread_(ui_thread),
       db_thread_(db_thread),
-      weak_ptr_factory_(this),
       autofill_backend_(new AutofillWebDataBackendImpl(NULL,
           ui_thread_,
           db_thread_,
-          base::Closure())) {
+          base::Closure())),
+      weak_ptr_factory_(this) {
 }
 
 void AutofillWebDataService::ShutdownOnUIThread() {

components/autofill/core/browser/webdata/autofill_webdata_service.h

@@ -118,12 +118,12 @@ class AutofillWebDataService : public AutofillWebData,
   // The MessageLoopProxy that this class uses as its DB thread.
   scoped_refptr<base::MessageLoopProxy> db_thread_;
 
+  scoped_refptr<AutofillWebDataBackendImpl> autofill_backend_;
+
   // This factory is used on the UI thread. All vended weak pointers are
   // invalidated in ShutdownOnUIThread().
   base::WeakPtrFactory<AutofillWebDataService> weak_ptr_factory_;
 
-  scoped_refptr<AutofillWebDataBackendImpl> autofill_backend_;
-
   DISALLOW_COPY_AND_ASSIGN(AutofillWebDataService);
 };
 

components/nacl/browser/nacl_browser.cc

@@ -137,8 +137,7 @@ base::File OpenNaClReadExecImpl(const base::FilePath& file_path,
 }
 
 NaClBrowser::NaClBrowser()
-    : weak_factory_(this),
-      irt_filepath_(),
+    : irt_filepath_(),
       irt_state_(NaClResourceUninitialized),
       validation_cache_file_path_(),
       validation_cache_is_enabled_(
@@ -147,7 +146,8 @@ NaClBrowser::NaClBrowser()
       validation_cache_is_modified_(false),
       validation_cache_state_(NaClResourceUninitialized),
       path_cache_(kFilePathCacheSize),
-      ok_(true) {
+      ok_(true),
+      weak_factory_(this) {
 }
 
 void NaClBrowser::SetDelegate(NaClBrowserDelegate* delegate) {

components/nacl/browser/nacl_browser.h

@@ -170,8 +170,6 @@ class NaClBrowser {
   void MarkValidationCacheAsModified();
   void PersistValidationCache();
 
-  // Singletons get destroyed at shutdown.
-  base::WeakPtrFactory<NaClBrowser> weak_factory_;
 
   base::File irt_file_;
   base::FilePath irt_filepath_;
@@ -200,6 +198,9 @@ class NaClBrowser {
 
   std::deque<base::Time> crash_times_;
 
+  // Singletons get destroyed at shutdown.
+  base::WeakPtrFactory<NaClBrowser> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(NaClBrowser);
 };
 

components/nacl/browser/nacl_process_host.cc

@@ -274,7 +274,6 @@ NaClProcessHost::NaClProcessHost(const GURL& manifest_url,
       debug_exception_handler_requested_(false),
 #endif
       internal_(new NaClInternal()),
-      weak_factory_(this),
       uses_irt_(uses_irt),
       uses_nonsfi_mode_(uses_nonsfi_mode),
       enable_debug_stub_(false),
@@ -283,7 +282,8 @@ NaClProcessHost::NaClProcessHost(const GURL& manifest_url,
       enable_crash_throttling_(enable_crash_throttling),
       off_the_record_(off_the_record),
       profile_directory_(profile_directory),
-      render_view_id_(render_view_id) {
+      render_view_id_(render_view_id),
+      weak_factory_(this) {
   process_.reset(content::BrowserChildProcessHost::Create(
       PROCESS_TYPE_NACL_LOADER, this));
 

components/nacl/browser/nacl_process_host.h

@@ -223,8 +223,6 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
   // Socket pairs for the NaCl process and renderer.
   scoped_ptr<NaClInternal> internal_;
 
-  base::WeakPtrFactory<NaClProcessHost> weak_factory_;
-
   scoped_ptr<content::BrowserChildProcessHost> process_;
 
   bool uses_irt_;
@@ -253,6 +251,8 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
   // reporting crash information.
   base::SharedMemory crash_info_shmem_;
 
+  base::WeakPtrFactory<NaClProcessHost> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(NaClProcessHost);
 };
 

components/policy/core/common/async_policy_loader.h

@@ -99,9 +99,6 @@ class POLICY_EXPORT AsyncPolicyLoader {
   // Callback for updates, passed in Init().
   UpdateCallback update_callback_;
 
-  // Used to get WeakPtrs for the periodic reload task.
-  base::WeakPtrFactory<AsyncPolicyLoader> weak_factory_;
-
   // Records last known modification timestamp.
   base::Time last_modification_time_;
 
@@ -114,6 +111,9 @@ class POLICY_EXPORT AsyncPolicyLoader {
   // The current policy schemas that this provider should load.
   scoped_refptr<SchemaMap> schema_map_;
 
+  // Used to get WeakPtrs for the periodic reload task.
+  base::WeakPtrFactory<AsyncPolicyLoader> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(AsyncPolicyLoader);
 };
 

components/policy/core/common/cloud/user_cloud_policy_store.cc

@@ -168,10 +168,10 @@ UserCloudPolicyStore::UserCloudPolicyStore(
     const std::string& verification_key,
     scoped_refptr<base::SequencedTaskRunner> background_task_runner)
     : UserCloudPolicyStoreBase(background_task_runner),
-      weak_factory_(this),
       policy_path_(policy_path),
       key_path_(key_path),
-      verification_key_(verification_key) {}
+      verification_key_(verification_key),
+      weak_factory_(this) {}
 
 UserCloudPolicyStore::~UserCloudPolicyStore() {}
 

components/policy/core/common/cloud/user_cloud_policy_store.h

@@ -91,9 +91,6 @@ class POLICY_EXPORT UserCloudPolicyStore : public UserCloudPolicyStoreBase {
   // Callback invoked to store the policy after validation has finished.
   void StorePolicyAfterValidation(UserCloudPolicyValidator* validator);
 
-  // WeakPtrFactory used to create callbacks for validating and storing policy.
-  base::WeakPtrFactory<UserCloudPolicyStore> weak_factory_;
-
   // The key used to verify signatures of cached policy.
   std::string policy_key_;
 
@@ -106,6 +103,9 @@ class POLICY_EXPORT UserCloudPolicyStore : public UserCloudPolicyStoreBase {
   // The hard-coded key used to verify new signing keys.
   const std::string verification_key_;
 
+  // WeakPtrFactory used to create callbacks for validating and storing policy.
+  base::WeakPtrFactory<UserCloudPolicyStore> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(UserCloudPolicyStore);
 };
 

components/signin/core/browser/signin_manager.cc

@@ -63,9 +63,9 @@ SigninManager::SigninManager(SigninClient* client,
     : SigninManagerBase(client),
       prohibit_signout_(false),
       type_(SIGNIN_TYPE_NONE),
-      weak_pointer_factory_(this),
       client_(client),
-      token_service_(token_service) {}
+      token_service_(token_service),
+      weak_pointer_factory_(this) {}
 
 void SigninManager::AddMergeSessionObserver(
     MergeSessionHelper::Observer* observer) {

components/signin/core/browser/signin_manager.h

@@ -193,8 +193,6 @@ class SigninManager : public SigninManagerBase {
   // token service so that it does not need to mint new ones.
   std::string temp_refresh_token_;
 
-  base::WeakPtrFactory<SigninManager> weak_pointer_factory_;
-
   // The SigninClient object associated with this object. Must outlive this
   // object.
   SigninClient* client_;
@@ -213,6 +211,8 @@ class SigninManager : public SigninManagerBase {
   // Helper to merge signed in account into the content area.
   scoped_ptr<MergeSessionHelper> merge_session_helper_;
 
+  base::WeakPtrFactory<SigninManager> weak_pointer_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(SigninManager);
 };
 

components/storage_monitor/portable_device_watcher_win.h

@@ -132,12 +132,12 @@ class PortableDeviceWatcherWin {
   // should not be performed on the UI thread.
   scoped_refptr<base::SequencedTaskRunner> media_task_runner_;
 
-  // Used by |media_task_runner_| to create cancelable callbacks.
-  base::WeakPtrFactory<PortableDeviceWatcherWin> weak_ptr_factory_;
-
   // The notifications object to use to signal newly attached devices.
   StorageMonitor::Receiver* storage_notifications_;
 
+  // Used by |media_task_runner_| to create cancelable callbacks.
+  base::WeakPtrFactory<PortableDeviceWatcherWin> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(PortableDeviceWatcherWin);
 };
 

components/suggestions/suggestions_service.cc

@@ -127,8 +127,8 @@ SuggestionsService::SuggestionsService(
       thumbnail_manager_(thumbnail_manager.Pass()),
       url_request_context_(url_request_context),
       blacklist_delay_sec_(kBlacklistDefaultDelaySec),
-      weak_ptr_factory_(this),
-      request_timeout_ms_(kDefaultRequestTimeoutMs) {
+      request_timeout_ms_(kDefaultRequestTimeoutMs),
+      weak_ptr_factory_(this) {
   // Obtain various parameters from Variations.
   suggestions_url_ =
       GURL(GetExperimentParam(kSuggestionsFieldTrialURLParam) + "?" +

components/suggestions/suggestions_service.h

@@ -188,13 +188,13 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
   // Delay used when scheduling a blacklisting task.
   int blacklist_delay_sec_;
 
-  // For callbacks may be run after destruction.
-  base::WeakPtrFactory<SuggestionsService> weak_ptr_factory_;
-
   // Timeout (in ms) before serving requestors after a fetch suggestions request
   // has been issued.
   int request_timeout_ms_;
 
+  // For callbacks may be run after destruction.
+  base::WeakPtrFactory<SuggestionsService> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(SuggestionsService);
 };
 

components/sync_driver/model_association_manager.cc

@@ -114,8 +114,8 @@ ModelAssociationManager::ModelAssociationManager(
     : state_(IDLE),
       controllers_(controllers),
       delegate_(processor),
-      weak_ptr_factory_(this),
-      configure_status_(DataTypeManager::UNKNOWN) {
+      configure_status_(DataTypeManager::UNKNOWN),
+      weak_ptr_factory_(this) {
   // Ensure all data type controllers are stopped.
   for (DataTypeController::TypeMap::const_iterator it = controllers_->begin();
        it != controllers_->end(); ++it) {

components/sync_driver/model_association_manager.h

@@ -146,10 +146,10 @@ class ModelAssociationManager {
   // Timer to track and limit how long a datatype takes to model associate.
   base::OneShotTimer<ModelAssociationManager> timer_;
 
-  base::WeakPtrFactory<ModelAssociationManager> weak_ptr_factory_;
-
   DataTypeManager::ConfigureStatus configure_status_;
 
+  base::WeakPtrFactory<ModelAssociationManager> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ModelAssociationManager);
 };