Notify DataUseTabModel of navigations and tab closures

chrome/browser/android/data_usage/data_use_tab_model.h

Index: chrome/browser/android/data_usage/data_use_tab_model.h
diff --git a/chrome/browser/android/data_usage/data_use_tab_model.h b/chrome/browser/android/data_usage/data_use_tab_model.h
index 43a07bdb110c5a8ac329cb20af38617e10afe9e3..8ca7c62bfb3a4d92b4e7dbed2da9aba912de9d82 100644
--- a/chrome/browser/android/data_usage/data_use_tab_model.h
+++ b/chrome/browser/android/data_usage/data_use_tab_model.h
@@ -79,16 +79,16 @@ class DataUseTabModel {
 
   ~DataUseTabModel();
 
-  base::WeakPtr<DataUseTabModel> GetWeakPtr();
+  // Returns weak pointers for use on IO and UI thread, respectively.
+  base::WeakPtr<DataUseTabModel> GetIOWeakPtr();
+  base::WeakPtr<DataUseTabModel> GetUIWeakPtr();

[sclittle, 2015/11/16 23:13:42]

WeakPtrs aren't safe to be used on a different thread than the one that the
object lives on. See my comment on the WeakPtrFactories below.

[tbansal1, 2015/11/17 21:12:33]

Done.

 
   // Notifies the DataUseTabModel of navigation events. |tab_id| is the source
   // tab of the generated event, |transition| indicates the type of the UI
-  // event/transition,  |url| is the URL in the source tab, |package| indicates
-  // the android package name of external application that initiated the event.
+  // event/transition,  |url| is the URL in the source tab.
   void OnNavigationEvent(int32_t tab_id,
                          TransitionType transition,
-                         const GURL& url,
-                         const std::string& package);
+                         const GURL& url);
 
   // Notifies the DataUseTabModel that tab with |tab_id| is closed. Any active
   // tracking sessions for the tab are terminated, and the tab is marked as
@@ -160,9 +160,10 @@ class DataUseTabModel {
 
   base::ThreadChecker thread_checker_;
 
-  // |weak_factory_| is used for generating weak pointers to be passed to
-  // DataUseTabUIManager on the UI thread.
-  base::WeakPtrFactory<DataUseTabModel> weak_factory_;
+  // |io_weak_factory_| and |ui_weak_factory_| are used for posting tasks on the
+  // IO and UI thread, respectively.
+  base::WeakPtrFactory<DataUseTabModel> io_weak_factory_;
+  base::WeakPtrFactory<DataUseTabModel> ui_weak_factory_;

[sclittle, 2015/11/16 23:13:42]

Having WeakPtrFactories for different threads isn't safe, since WeakPtrFactories
aren't thread safe.

E.g., the DataUseTabModel could be destroyed on the IO thread while some UI
thread code is in the process of using the DataUseTabModel, even though the
WeakPtr was valid earlier.

The simplest solution is usually to have an object live on only one thread, and
always PostTask to that thread when dealing with that object. One way would be
to split up the IO and UI functionality of this class into different classes.

[tbansal1, 2015/11/17 21:12:33]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(DataUseTabModel);
 };