Remove one thread hop while fetching matching rules

chrome/browser/android/data_usage/external_data_use_observer.cc

Index: chrome/browser/android/data_usage/external_data_use_observer.cc
diff --git a/chrome/browser/android/data_usage/external_data_use_observer.cc b/chrome/browser/android/data_usage/external_data_use_observer.cc
index a1dba79a73f3b54b3b09cc35fe3d352f6c4ec5ab..abc12ba527024008468140caeb9316244e3a2d0f 100644
--- a/chrome/browser/android/data_usage/external_data_use_observer.cc
+++ b/chrome/browser/android/data_usage/external_data_use_observer.cc
@@ -162,12 +162,11 @@ ExternalDataUseObserver::ExternalDataUseObserver(
   ui_task_runner_->PostTask(FROM_HERE,
                             base::Bind(&DataUseTabModel::InitOnUIThread,
                                        base::Unretained(data_use_tab_model_),
-                                       io_task_runner, GetWeakPtr()));
+                                       external_data_use_observer_bridge_));
 
-  // Initialize the ExternalDataUseObserverBridge object. Initialization will
-  // also trigger the fetching of matching rules. It is okay to use
-  // base::Unretained here since |external_data_use_observer_bridge_| is
-  // owned by |this|, and is destroyed on UI thread when |this| is destroyed.
+  // Initialize the ExternalDataUseObserverBridge object. It is okay to use
+  // base::Unretained here since |external_data_use_observer_bridge_| is owned
+  // by |this|, and is destroyed on UI thread when |this| is destroyed.
   ui_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&ExternalDataUseObserverBridge::Init,
@@ -181,17 +180,18 @@ ExternalDataUseObserver::~ExternalDataUseObserver() {
   if (registered_as_data_use_observer_)
     data_use_aggregator_->RemoveObserver(this);
 
+  // Delete |data_use_tab_model_| on the UI thread. |data_use_tab_model_| should
+  // be deleted before |external_data_use_observer_bridge_|.
+  if (!ui_task_runner_->DeleteSoon(FROM_HERE, data_use_tab_model_)) {
+    NOTIMPLEMENTED() << " DataUseTabModel was not deleted successfully";
+  }
+
   // Delete |external_data_use_observer_bridge_| on the UI thread.
   if (!ui_task_runner_->DeleteSoon(FROM_HERE,
                                    external_data_use_observer_bridge_)) {
     NOTIMPLEMENTED()
         << " ExternalDataUseObserverBridge was not deleted successfully";
   }
-
-  // Delete |data_use_tab_model_| on the UI thread.
-  if (!ui_task_runner_->DeleteSoon(FROM_HERE, data_use_tab_model_)) {
-    NOTIMPLEMENTED() << " DataUseTabModel was not deleted successfully";
-  }
 }
 
 void ExternalDataUseObserver::OnReportDataUseDone(bool success) {
@@ -281,6 +281,10 @@ void ExternalDataUseObserver::FetchMatchingRules() {
                  base::Unretained(external_data_use_observer_bridge_)));
 }
 
+void ExternalDataUseObserver::OnFetchMatchingRulesDone() {

[Raj, 2016/03/09 02:27:36]

I am more inclined to move the 15 minute rule fetch logic to DataUseTabModel.
Then:
 * last_matching_rules_fetch_time_ will be directly updated in tab model.
 * Fetch can happen based on whether control app is installed or not.
 * all the fetch logic will be in one place.
WDYT.

[tbansal1, 2016/03/10 01:00:58]

As discussed offline, I would suggest thinking of how you can re-architecture
all the classes so that it is easier to handle different kinds of data use
tracking. If DataUseTabModel is going to contain one model of data use tracking,
then probably it should not be doing general stuff.

[Raj, 2016/03/10 02:13:34]

Acknowledged.

+  last_matching_rules_fetch_time_ = base::TimeTicks::Now();
+}
+
 void ExternalDataUseObserver::DataUseLabelApplied(
     const data_usage::DataUse& data_use,
     const base::Time& start_time,