Declaring the weak_ptr_factory in proper order in src/components

Cleaning up weak_ptr_factory destruction order. WeakPtrFactory should remain the last member so it'll be destroyed and invalidate its weak pointers before any other members are destroyed. BUG=303818 Review URL: https://codereview.chromium.org/630983003 Cr-Commit-Position: refs/heads/master@{#298842}
280723148 · Oct 9, 2014 · 4c90c51 · 4c90c51
1 parent 3d57cba
commit 4c90c51
Show file tree

Hide file tree

Showing 9 changed files with 28 additions and 27 deletions.
diff --git a/AUTHORS b/AUTHORS
@@ -347,6 +347,7 @@ Philippe Beauchamp <philippe.beauchamp@gmail.com>
 Philippe Beaudoin <philippe.beaudoin@gmail.com>
 Pierre-Antoine LaFayette <pierre.lafayette@gmail.com>
 Po-Chun Chang <pochang0403@gmail.com>
+Pramod Begur Srinath <pramod.bs@samsung.com>
 Prashant Hiremath <prashhir@cisco.com>
 Prashant Nevase <prashant.n@samsung.com>
 Praveen Akkiraju <praveen.anp@samsung.com>

diff --git a/components/autofill/content/browser/risk/fingerprint.cc b/components/autofill/content/browser/risk/fingerprint.cc
@@ -247,17 +247,17 @@ class FingerprintDataLoader : public content::GpuDataManagerObserver {
   // if not all asynchronous data has been loaded.
   base::OneShotTimer<FingerprintDataLoader> timeout_timer_;
 
-  // For invalidating asynchronous callbacks that might arrive after |this|
-  // instance is destroyed.
-  base::WeakPtrFactory<FingerprintDataLoader> weak_ptr_factory_;
-
   // The callback that will be called once all the data is available.
   base::Callback<void(scoped_ptr<Fingerprint>)> callback_;
 
   // The callback used as an "observer" of the GeolocationProvider.
   scoped_ptr<content::GeolocationProvider::Subscription>
       geolocation_subscription_;
 
+  // For invalidating asynchronous callbacks that might arrive after |this|
+  // instance is destroyed.
+  base::WeakPtrFactory<FingerprintDataLoader> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(FingerprintDataLoader);
 };
 
@@ -287,8 +287,8 @@ FingerprintDataLoader::FingerprintDataLoader(
       user_agent_(user_agent),
       install_time_(install_time),
       waiting_on_plugins_(true),
-      weak_ptr_factory_(this),
-      callback_(callback) {
+      callback_(callback),
+      weak_ptr_factory_(this) {
   DCHECK(!install_time_.is_null());
 
   timeout_timer_.Start(FROM_HERE, timeout,

diff --git a/components/autofill/core/browser/autofill_external_delegate.cc b/components/autofill/core/browser/autofill_external_delegate.cc
@@ -50,8 +50,8 @@ AutofillExternalDelegate::AutofillExternalDelegate(AutofillManager* manager,
       display_warning_if_disabled_(false),
       has_suggestion_(false),
       has_shown_popup_for_current_edit_(false),
-      weak_ptr_factory_(this),
-      has_shown_address_book_prompt(false) {
+      has_shown_address_book_prompt(false),
+      weak_ptr_factory_(this) {
   DCHECK(manager);
 }
 

diff --git a/components/autofill/core/browser/autofill_external_delegate.h b/components/autofill/core/browser/autofill_external_delegate.h
@@ -150,12 +150,12 @@ class AutofillExternalDelegate
   std::vector<base::string16> data_list_values_;
   std::vector<base::string16> data_list_labels_;
 
-  base::WeakPtrFactory<AutofillExternalDelegate> weak_ptr_factory_;
-
   // Whether the access Address Book prompt has ever been shown for the current
   // |query_form_|. This variable is only used on OSX.
   bool has_shown_address_book_prompt;
 
+  base::WeakPtrFactory<AutofillExternalDelegate> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(AutofillExternalDelegate);
 };
 

diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_statistics_prefs.cc b/components/data_reduction_proxy/browser/data_reduction_proxy_statistics_prefs.cc
@@ -22,9 +22,9 @@ DataReductionProxyStatisticsPrefs::DataReductionProxyStatisticsPrefs(
     const base::TimeDelta& delay)
     : pref_service_(prefs),
       task_runner_(task_runner),
-      weak_factory_(this),
       delay_(delay),
-      delayed_task_posted_(false) {
+      delayed_task_posted_(false),
+      weak_factory_(this) {
   DCHECK(prefs);
   DCHECK_GE(delay.InMilliseconds(), 0);
   Init();

diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_statistics_prefs.h b/components/data_reduction_proxy/browser/data_reduction_proxy_statistics_prefs.h
@@ -87,11 +87,11 @@ class DataReductionProxyStatisticsPrefs {
 
   PrefService* pref_service_;
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
-  base::WeakPtrFactory<DataReductionProxyStatisticsPrefs> weak_factory_;
   const base::TimeDelta delay_;
   bool delayed_task_posted_;
   DataReductionProxyPrefMap pref_map_;
   DataReductionProxyListPrefMap list_pref_map_;
+  base::WeakPtrFactory<DataReductionProxyStatisticsPrefs> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyStatisticsPrefs);
 };

diff --git a/components/enhanced_bookmarks/enhanced_bookmark_model.cc b/components/enhanced_bookmarks/enhanced_bookmark_model.cc
@@ -81,8 +81,8 @@ EnhancedBookmarkModel::EnhancedBookmarkModel(BookmarkModel* bookmark_model,
                                              const std::string& version)
     : bookmark_model_(bookmark_model),
       loaded_(false),
-      weak_ptr_factory_(this),
-      version_(version) {
+      version_(version),
+      weak_ptr_factory_(this) {
   bookmark_model_->AddObserver(this);
   if (bookmark_model_->loaded()) {
     InitializeIdMap();

diff --git a/components/enhanced_bookmarks/enhanced_bookmark_model.h b/components/enhanced_bookmarks/enhanced_bookmark_model.h
@@ -187,8 +187,6 @@ class EnhancedBookmarkModel : public KeyedService,
 
   ObserverList<EnhancedBookmarkModelObserver> observers_;
 
-  base::WeakPtrFactory<EnhancedBookmarkModel> weak_ptr_factory_;
-
   IdToNodeMap id_map_;
   NodeToIdMap nodes_to_reset_;
 
@@ -197,6 +195,8 @@ class EnhancedBookmarkModel : public KeyedService,
 
   std::string version_;
   std::string version_suffix_;
+
+  base::WeakPtrFactory<EnhancedBookmarkModel> weak_ptr_factory_;
 };
 
 }  // namespace enhanced_bookmarks

diff --git a/components/metrics/metrics_service.h b/components/metrics/metrics_service.h
@@ -244,6 +244,8 @@ class MetricsService : public base::HistogramFlattener {
     NEED_TO_SHUTDOWN = ~CLEANLY_SHUTDOWN
   };
 
+  friend class ::MetricsServiceAccessor;
+
   typedef std::vector<SyntheticTrialGroup> SyntheticTrialGroups;
 
   // Calls into the client to start metrics gathering.
@@ -433,14 +435,6 @@ class MetricsService : public base::HistogramFlattener {
   // A number that identifies the how many times the app has been launched.
   int session_id_;
 
-  // Weak pointers factory used to post task on different threads. All weak
-  // pointers managed by this factory have the same lifetime as MetricsService.
-  base::WeakPtrFactory<MetricsService> self_ptr_factory_;
-
-  // Weak pointers factory used for saving state. All weak pointers managed by
-  // this factory are invalidated in ScheduleNextStateSave.
-  base::WeakPtrFactory<MetricsService> state_saver_factory_;
-
   // The scheduler for determining when uploads should happen.
   scoped_ptr<MetricsReportingScheduler> scheduler_;
 
@@ -460,13 +454,19 @@ class MetricsService : public base::HistogramFlattener {
   // exited-cleanly bit in the prefs.
   static ShutdownCleanliness clean_shutdown_status_;
 
-  friend class ::MetricsServiceAccessor;
-
   FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest, IsPluginProcess);
   FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest,
                            PermutedEntropyCacheClearedWhenLowEntropyReset);
   FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest, RegisterSyntheticTrial);
 
+  // Weak pointers factory used to post task on different threads. All weak
+  // pointers managed by this factory have the same lifetime as MetricsService.
+  base::WeakPtrFactory<MetricsService> self_ptr_factory_;
+
+  // Weak pointers factory used for saving state. All weak pointers managed by
+  // this factory are invalidated in ScheduleNextStateSave.
+  base::WeakPtrFactory<MetricsService> state_saver_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(MetricsService);
 };