Notify DataUseTabModel of navigations and tab closures

chrome/browser/android/data_usage/data_use_ui_tab_model.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/android/data_usage/data_use_ui_tab_model.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
@@ skipping 13 matching lines @@
 
 DataUseUITabModel::~DataUseUITabModel() {}
 
 void DataUseUITabModel::ReportBrowserNavigation(
     const GURL& gurl,
     ui::PageTransition page_transition,
     int32_t tab_id) const {
   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);
   if (it == tab_events_.end())
     return;
   tab_events_.erase(it);
 }
 
 void DataUseUITabModel::OnTrackingStarted(int32_t tab_id) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
@@ skipping 53 matching lines @@
                                        DataUseTrackingEvent event) {
   TabEvents::iterator it = tab_events_.find(tab_id);
   DCHECK(it != tab_events_.end());
   if (it->second == event) {
     tab_events_.erase(it);
     return true;
   }
   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