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 95e86286088001fabe436cda92c6f9375ccb4a8d..aefc054f402f0dbf2e621ecfaef9b5fcba897530 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
@@ -8,7 +8,7 @@
 
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
-#include "base/single_thread_task_runner.h"
+#include "base/thread_task_runner_handle.h"
 #include "content/public/browser/browser_thread.h"
 #include "url/gurl.h"
 
@@ -16,29 +16,93 @@ namespace chrome {
 
 namespace android {
 
+// TabObserverOnIOThread registers as an observer to
+// DataUseTabModel::TabDataUseObserver, and gets notified when tracking has
+// started and ended on a tab. It passes these notifications to
+// DataUseUITabModel on UI thread.  TabObserverOnIOThread is not thread safe,
+// and should only be accessed only on IO thread.
+class DataUseUITabModel::TabObserverOnIOThread
+    : public base::RefCountedThreadSafe<TabObserverOnIOThread>,
+      public DataUseTabModel::TabDataUseObserver {
+ public:
+  // Sets the |data_use_tab_model_|, and registers as an observer to
+  // |data_use_tab_model_|. Can be called at most once.
+  TabObserverOnIOThread(

[sclittle, 2015/11/19 21:53:24]

Maybe this class isn't necessary. What if DataUseTabModel just had a map of
WeakPtr<DataUseUITabModel>s and each DataUseUITabModel just registered itself
directly with its own WeakPtr? Then, both classes could just PostTasks at each
other to communicate.

You'd probably want to key the map like std::map<const DataUseUITabModel*,
WeakPtr<DataUseUITabModel>> so that the DataUseUITabModels can remove themselves
as observers.

It would be safe to use a regular std::map instead of a special ObserverList
because all notifications would happen asyncronously with PostTask, so it would
be impossible for the map to change while it's being iterated over.

[tbansal1, 2015/11/20 21:34:05]

Done.

+      const base::WeakPtr<DataUseUITabModel>& data_use_ui_tab_model,
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
+      const base::WeakPtr<DataUseTabModel>& data_use_tab_model);
+
+ private:
+  friend class DataUseUITabModelTest;

[sclittle, 2015/11/19 21:53:24]

This friending does nothing, so remove it.

[tbansal1, 2015/11/20 21:34:05]

Done.

+
+  // Ref counted classes have private destructors to avoid any code deleting
+  // the object accidentally while there are still references to it.
+  friend class base::RefCountedThreadSafe<TabObserverOnIOThread>;
+
+  ~TabObserverOnIOThread();
+
+  // DataUseTabModel::Observer implementation:
+  void NotifyTrackingStarting(int32_t tab_id) override;
+  void NotifyTrackingEnding(int32_t tab_id) override;
+
+  // |data_use_ui_tab_model_| is notified of starting and ending of tracking.
+  // |data_use_ui_tab_model_| should only be used on UI thread.
+  base::WeakPtr<DataUseUITabModel> data_use_ui_tab_model_;
+
+  // |this| registers as an observer to |data_use_tab_model_|.
+  base::WeakPtr<DataUseTabModel> data_use_tab_model_;
+
+  // Used to call methods on |data_use_ui_tab_model_|.
+  scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
+
+  base::ThreadChecker thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(TabObserverOnIOThread);
+};
+
 DataUseUITabModel::DataUseUITabModel(
     scoped_refptr<base::SingleThreadTaskRunner> io_task_runner)
-    : io_task_runner_(io_task_runner) {
+    : io_task_runner_(io_task_runner), weak_factory_(this) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(io_task_runner_);
 }
 
-DataUseUITabModel::~DataUseUITabModel() {}
+DataUseUITabModel::~DataUseUITabModel() {
+  if (tab_data_use_observer_) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    tab_data_use_observer_->AddRef();
+
+    // Removes the last reference to |tab_observer|, so that it is destroyed on
+    // IO thread.
+    io_task_runner_->ReleaseSoon(FROM_HERE, tab_data_use_observer_.get());

[sclittle, 2015/11/19 21:53:24]

I don't think this is safe - ReleaseSoon could fail if the other thread doesn't
exist anymore, see
https://code.google.com/p/chromium/codesearch#chromium/src/base/sequenced_tas...

It would be simpler to just remove the observer class altogether like I
commented above. Alternatively you could have the IOTabModel own the observers,
but at that point you might as well just link the IOTabModel with the UITabModel
directly without the observer.

[tbansal1, 2015/11/20 21:34:05]

Done.

+    tab_data_use_observer_ = nullptr;
+  }
+}
 
 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());
+  DCHECK_LE(0, tab_id);
 
-  // TODO(tbansal): Post to DataUseTabModel on IO thread.
+  DataUseTabModel::TransitionType transition_type;
+
+  if (ConvertTransitionType(page_transition, &transition_type)) {
+    io_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&DataUseTabModel::OnNavigationEvent, io_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());
+  DCHECK_LE(0, tab_id);
 
-  // TODO(tbansal): Post to DataUseTabModel on IO thread.
+  io_task_runner_->PostTask(FROM_HERE,
+                            base::Bind(&DataUseTabModel::OnTabCloseEvent,
+                                       io_data_use_tab_model_, tab_id));
 
   // Clear out local state.
   TabEvents::iterator it = tab_events_.find(tab_id);
@@ -51,11 +115,27 @@ void DataUseUITabModel::ReportCustomTabInitialNavigation(
     int32_t tab_id,
     const std::string& url,
     const std::string& package_name) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // TODO(tbansal): Post to DataUseTabModel on IO thread.
 }
 
-void DataUseUITabModel::OnTrackingStarted(int32_t tab_id) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+void DataUseUITabModel::SetIODataUseTabModel(
+    const base::WeakPtr<DataUseTabModel>& io_data_use_tab_model) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!io_data_use_tab_model_);
+
+  io_data_use_tab_model_ = io_data_use_tab_model;
+
+  base::PostTaskAndReplyWithResult(
+      io_task_runner_.get(), FROM_HERE,
+      base::Bind(&CreateTabObserverOnIOThread, io_data_use_tab_model_,
+                 weak_factory_.GetWeakPtr(),
+                 base::ThreadTaskRunnerHandle::Get()),
+      base::Bind(&DataUseUITabModel::SetTabDataUseObserver,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void DataUseUITabModel::NotifyTrackingStarting(int32_t tab_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (MaybeCreateTabEvent(tab_id, DATA_USE_TRACKING_STARTED))
@@ -65,8 +145,7 @@ void DataUseUITabModel::OnTrackingStarted(int32_t tab_id) {
   RemoveTabEvent(tab_id, DATA_USE_TRACKING_ENDED);
 }
 
-void DataUseUITabModel::OnTrackingEnded(int32_t tab_id) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+void DataUseUITabModel::NotifyTrackingEnding(int32_t tab_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (MaybeCreateTabEvent(tab_id, DATA_USE_TRACKING_ENDED))
@@ -77,7 +156,6 @@ void DataUseUITabModel::OnTrackingEnded(int32_t tab_id) {
 }
 
 bool DataUseUITabModel::HasDataUseTrackingStarted(int32_t tab_id) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(thread_checker_.CalledOnValidThread());
 
   TabEvents::iterator it = tab_events_.find(tab_id);
@@ -88,7 +166,6 @@ bool DataUseUITabModel::HasDataUseTrackingStarted(int32_t tab_id) {
 }
 
 bool DataUseUITabModel::HasDataUseTrackingEnded(int32_t tab_id) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(thread_checker_.CalledOnValidThread());
 
   TabEvents::iterator it = tab_events_.find(tab_id);
@@ -98,18 +175,37 @@ bool DataUseUITabModel::HasDataUseTrackingEnded(int32_t tab_id) {
   return RemoveTabEvent(tab_id, DATA_USE_TRACKING_ENDED);
 }
 
+// static
+scoped_refptr<DataUseUITabModel::TabObserverOnIOThread>
+DataUseUITabModel::CreateTabObserverOnIOThread(
+    const base::WeakPtr<DataUseTabModel>& data_use_tab_model,
+    const base::WeakPtr<DataUseUITabModel>& data_use_ui_tab_model,
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+
+  scoped_refptr<DataUseUITabModel::TabObserverOnIOThread> tab_observer(
+      new DataUseUITabModel::TabObserverOnIOThread(
+          data_use_ui_tab_model, ui_task_runner, data_use_tab_model));
+  return tab_observer;
+}
+
+void DataUseUITabModel::SetTabDataUseObserver(
+    scoped_refptr<TabObserverOnIOThread> tab_data_use_observer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!tab_data_use_observer_);
+  tab_data_use_observer_ = tab_data_use_observer;
+  DCHECK(tab_data_use_observer_);
+}
+
 bool DataUseUITabModel::MaybeCreateTabEvent(int32_t tab_id,
                                             DataUseTrackingEvent event) {
-  TabEvents::iterator it = tab_events_.find(tab_id);
-  if (it == tab_events_.end()) {
-    tab_events_.insert(std::make_pair(tab_id, event));
-    return true;
-  }
-  return false;
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return tab_events_.insert(std::make_pair(tab_id, event)).second;
 }
 
 bool DataUseUITabModel::RemoveTabEvent(int32_t tab_id,
                                        DataUseTrackingEvent event) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   TabEvents::iterator it = tab_events_.find(tab_id);
   DCHECK(it != tab_events_.end());
   if (it->second == event) {
@@ -119,6 +215,85 @@ bool DataUseUITabModel::RemoveTabEvent(int32_t tab_id,
   return false;
 }
 
+bool DataUseUITabModel::ConvertTransitionType(
+    ui::PageTransition page_transition,
+    DataUseTabModel::TransitionType* transition_type) const {
+  if (!ui::PageTransitionIsValidType(page_transition) ||
+      !ui::PageTransitionIsMainFrame(page_transition) ||
+      !ui::PageTransitionIsNewNavigation(page_transition)) {
+    return false;
+  }
+
+  const int32_t mask = 0xFFFFFFFF ^ ui::PAGE_TRANSITION_QUALIFIER_MASK;
+
+  switch (page_transition & mask) {
+    case ui::PAGE_TRANSITION_LINK:
+      if ((page_transition & ui::PAGE_TRANSITION_FROM_API) != 0) {
+        // Clicking on bookmarks.
+        *transition_type = DataUseTabModel::TRANSITION_BOOKMARK;
+        return true;
+      }
+      return false;  // Newtab, clicking on a link.
+    case ui::PAGE_TRANSITION_TYPED:
+      *transition_type = DataUseTabModel::TRANSITION_OMNIBOX_NAVIGATION;
+      return true;
+    case ui::PAGE_TRANSITION_AUTO_BOOKMARK:
+      // Auto bookmark from newtab page.
+      *transition_type = DataUseTabModel::TRANSITION_BOOKMARK;
+      return true;
+    case ui::PAGE_TRANSITION_AUTO_TOPLEVEL:
+      // History menu.
+      *transition_type = DataUseTabModel::TRANSITION_HISTORY_ITEM;
+      return true;
+    case ui::PAGE_TRANSITION_GENERATED:
+      // Omnibox search (e.g., searching for "tacos").
+      *transition_type = DataUseTabModel::TRANSITION_OMNIBOX_SEARCH;
+      return true;
+    case ui::PAGE_TRANSITION_RELOAD:
+      // Restored tabs.
+      return false;
+    default:
+      return false;
+  }
+}
+
+DataUseUITabModel::TabObserverOnIOThread::TabObserverOnIOThread(
+    const base::WeakPtr<DataUseUITabModel>& data_use_ui_tab_model,
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
+    const base::WeakPtr<DataUseTabModel>& data_use_tab_model)
+    : data_use_ui_tab_model_(data_use_ui_tab_model),
+      data_use_tab_model_(data_use_tab_model),
+      ui_task_runner_(ui_task_runner) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+  DCHECK(ui_task_runner_);
+  if (data_use_tab_model_)
+    data_use_tab_model_->AddObserver(this);
+}
+
+DataUseUITabModel::TabObserverOnIOThread::~TabObserverOnIOThread() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (data_use_tab_model_)
+    data_use_tab_model_->RemoveObserver(this);
+}
+
+void DataUseUITabModel::TabObserverOnIOThread::NotifyTrackingStarting(
+    int32_t tab_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  ui_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&DataUseUITabModel::NotifyTrackingStarting,
+                            data_use_ui_tab_model_, tab_id));
+}
+
+void DataUseUITabModel::TabObserverOnIOThread::NotifyTrackingEnding(
+    int32_t tab_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  ui_task_runner_->PostTask(FROM_HERE,
+                            base::Bind(&DataUseUITabModel::NotifyTrackingEnding,
+                                       data_use_ui_tab_model_, tab_id));
+}
+
 }  // namespace android
 
 }  // namespace chrome