Notify DataUseTabModel of navigations and tab closures

chrome/browser/android/data_usage/data_use_ui_tab_model.cc

Index: chrome/browser/android/data_usage/data_use_ui_tab_model.cc
diff --git a/chrome/browser/android/data_usage/data_use_ui_tab_model.cc b/chrome/browser/android/data_usage/data_use_ui_tab_model.cc
index 9b113a4227dc876939dc562507cd76655f048ec9..e3422986dbbbb6f6cd4f7c267e39800a03688ccb 100644
--- a/chrome/browser/android/data_usage/data_use_ui_tab_model.cc
+++ b/chrome/browser/android/data_usage/data_use_ui_tab_model.cc
@@ -31,14 +31,26 @@ void DataUseUITabModel::ReportBrowserNavigation(
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  // TODO(tbansal): Post to DataUseTabModel on IO thread.
+  if (data_use_tab_model_) {

[mmenke, 2015/11/13 22:49:55]

Is this legal?  You're checking if a weak ptr is NULL on a thread other than the
one it lives on.  Seems like at the very least, you could trigger a thread
sanitizer warning doing this.

[bengr, 2015/11/14 03:54:39]

This doesn't seem kosher. I think it should just post to the method on the weak
pointer.

[tbansal1, 2015/11/16 17:49:08]

Now using two weak pointers (one for UI, one for IO), and dereferencing them on
the correct threads.

[tbansal1, 2015/11/16 17:49:08]

Done.

+    DataUseTabModel::TransitionType transition_type;
+    if (ConvertTransitionType(page_transition, &transition_type)) {
+      io_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&DataUseTabModel::OnNavigationEvent, data_use_tab_model_,
+                     tab_id, transition_type, gurl));
+    }
+  }
 }
 
 void DataUseUITabModel::ReportTabClosure(int32_t tab_id) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  // TODO(tbansal): Post to DataUseTabModel on IO thread.
+  if (data_use_tab_model_) {

[bengr, 2015/11/14 03:54:39]

Same here.

[tbansal1, 2015/11/16 17:49:08]

Done.

+    io_task_runner_->PostTask(FROM_HERE,
+                              base::Bind(&DataUseTabModel::OnTabCloseEvent,
+                                         data_use_tab_model_, tab_id));
+  }
 
   // Clear out local state.
   TabEvents::iterator it = tab_events_.find(tab_id);
@@ -112,6 +124,25 @@ bool DataUseUITabModel::RemoveTabEvent(int32_t tab_id,
   return false;
 }
 
+bool DataUseUITabModel::ConvertTransitionType(
+    ui::PageTransition page_transition,
+    DataUseTabModel::TransitionType* transition_type) const {
+  int32_t mask = 0xFFFFFFFF ^ ui::PAGE_TRANSITION_QUALIFIER_MASK;
+  switch (page_transition & mask) {
+    case ui::PAGE_TRANSITION_TYPED:
+      *transition_type = DataUseTabModel::TRANSITION_OMNIBOX_NAVIGATION;
+      return true;
+    case ui::PAGE_TRANSITION_AUTO_BOOKMARK:

[bengr, 2015/11/14 03:54:39]

Where do we track navigation from history? Also, is the an exhaustive list?
Seems thin.

[tbansal1, 2015/11/16 17:49:08]

List should be exhaustive now.

+      *transition_type = DataUseTabModel::TRANSITION_BOOKMARK;
+      return true;
+    case ui::PAGE_TRANSITION_GENERATED:
+      *transition_type = DataUseTabModel::TRANSITION_OMNIBOX_SEARCH;
+      return true;
+    default:
+      return false;
+  }
+}
+
 }  // namespace android
 
 }  // namespace chrome