Remove one thread hop while fetching matching rules

chrome/browser/android/data_usage/data_use_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_tab_model.h"
 
 #include <stdint.h>
 
 #include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
@@ skipping 22 matching lines @@
 // overridden by the field trial.
 const uint32_t kDefaultClosedTabExpirationDurationSeconds = 30;  // 30 seconds.
 const uint32_t kDefaultOpenTabExpirationDurationSeconds =
     60 * 60 * 24 * 5;  // 5 days.
 
 // Default expiration duration in seconds of a matching rule, used when
 // expiration time is not specified.
 const uint32_t kDefaultMatchingRuleExpirationDurationSeconds =
     60 * 60 * 24;  // 24 hours.
 
+// Default maximum number of UI navigation events that are buffered initially
+// until they are processed later.
+const uint32_t kDefaultMaxNavigationEventsBuffered = 100;
+
 const char kUMAExpiredInactiveTabEntryRemovalDurationHistogram[] =
     "DataUsage.TabModel.ExpiredInactiveTabEntryRemovalDuration";
 const char kUMAExpiredActiveTabEntryRemovalDurationHistogram[] =
     "DataUsage.TabModel.ExpiredActiveTabEntryRemovalDuration";
 const char kUMAUnexpiredTabEntryRemovalDurationHistogram[] =
     "DataUsage.TabModel.UnexpiredTabEntryRemovalDuration";
 
 // Returns true if |tab_id| is a valid tab ID.
 bool IsValidTabID(SessionID::id_type tab_id) {
   return tab_id >= 0;
@@ skipping 73 matching lines @@
 namespace chrome {
 
 namespace android {
 
 DataUseTabModel::DataUseTabModel()
     : max_tab_entries_(GetMaxTabEntries()),
       max_sessions_per_tab_(GetMaxSessionsPerTab()),
       closed_tab_expiration_duration_(GetClosedTabExpirationDuration()),
       open_tab_expiration_duration_(GetOpenTabExpirationDuration()),
       is_control_app_installed_(false),
+      ui_navigation_buffer(new std::vector<DataUseUINavigationBuffer>()),
       weak_factory_(this) {
   // Detach from current thread since rest of DataUseTabModel lives on the UI
   // thread and the current thread may not be UI thread..
   thread_checker_.DetachFromThread();
 }
 
 DataUseTabModel::~DataUseTabModel() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
 base::WeakPtr<DataUseTabModel> DataUseTabModel::GetWeakPtr() {
   DCHECK(thread_checker_.CalledOnValidThread());
   return weak_factory_.GetWeakPtr();
 }
 
 void DataUseTabModel::InitOnUIThread(
-    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
-    const base::WeakPtr<ExternalDataUseObserver>& external_data_use_observer) {
+    const ExternalDataUseObserverBridge* external_data_use_observer_bridge) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(io_task_runner);
+  DCHECK(external_data_use_observer_bridge);
 
   tick_clock_.reset(new base::DefaultTickClock());
-  data_use_matcher_.reset(new DataUseMatcher(
-      GetWeakPtr(), io_task_runner, external_data_use_observer,
-      GetDefaultMatchingRuleExpirationDuration()));
+  data_use_matcher_.reset(
+      new DataUseMatcher(GetWeakPtr(), external_data_use_observer_bridge,
+                         GetDefaultMatchingRuleExpirationDuration()));
 }
 
 void DataUseTabModel::OnNavigationEvent(SessionID::id_type tab_id,
                                         TransitionType transition,
                                         const GURL& url,
                                         const std::string& package) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(IsValidTabID(tab_id));
   std::string current_label, new_label;
   bool is_package_match;

[tbansal1, 2016/03/08 18:15:00]

May be declare these variables after the if-block since they are not used in the
if-block.

 
+  if (ui_navigation_buffer) {
+    // Buffer the navigation event for later processing.
+    DataUseUINavigationBuffer ui_event;

[tbansal1, 2016/03/08 18:15:00]

If you want to force callers to fill all the fields of the struct, I would
suggest adding a constructor to the struct.

+    ui_event.tab_id = tab_id;
+    ui_event.transition = transition;
+    ui_event.url = url;
+    ui_event.package = package;
+    ui_navigation_buffer->push_back(ui_event);
+
+    if (ui_navigation_buffer->size() >= kDefaultMaxNavigationEventsBuffered) {

[tbansal1, 2016/03/08 18:15:00]

If the matching rules have not been fetched, what would be effect of this? It
seems that since there are no matching rules, none of the tabs will enter
tracking, and so processing would not have any effect.

May be add a comment to say what is expected to happen here.

+      ProcessBufferedNavigationEvents();
+    }
+    return;
+  }
+
   if (is_control_app_installed_ && !data_use_matcher_->HasValidRules())
     data_use_matcher_->FetchMatchingRules();
 
   GetCurrentAndNewLabelForNavigationEvent(tab_id, transition, url, package,
                                           &current_label, &new_label,
                                           &is_package_match);
   if (!current_label.empty() && new_label.empty()) {
     EndTrackingDataUse(tab_id);
   } else if (current_label.empty() && !new_label.empty()) {
     StartTrackingDataUse(
@@ skipping 69 matching lines @@
     const std::vector<std::string>& label) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Fetch rule requests could be started when control app was installed, and
   // the response could be received when control app was uninstalled. Ignore
   // these spurious rule fetch responses.
   if (!is_control_app_installed_)
     return;
   data_use_matcher_->RegisterURLRegexes(app_package_name, domain_path_regex,
                                         label);
+  ProcessBufferedNavigationEvents();
 }
 
 void DataUseTabModel::OnControlAppInstallStateChange(
     bool is_control_app_installed) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_NE(is_control_app_installed_, is_control_app_installed);
-  // If external control app is installed now, rules will be fetched on the next
-  // navigation event.
+
   is_control_app_installed_ = is_control_app_installed;
   std::vector<std::string> empty;
-  if (!is_control_app_installed_)  // Clear rules.
+  if (!is_control_app_installed_) {  // Clear rules.

[tbansal1, 2016/03/08 18:15:00]

nit: move the comment on next line?

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

Done.

     data_use_matcher_->RegisterURLRegexes(empty, empty, empty);
+    ProcessBufferedNavigationEvents();
+  } else {
+    data_use_matcher_->FetchMatchingRules();

[tbansal1, 2016/03/08 18:15:00]

Can we do this in a separate CL and merge it to M-50?

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

Done.

+  }
 }
 
 base::TimeTicks DataUseTabModel::NowTicks() const {
   DCHECK(thread_checker_.CalledOnValidThread());
   return tick_clock_->NowTicks();
 }
 
 bool DataUseTabModel::IsCustomTabPackageMatch(SessionID::id_type tab_id) const {
   DCHECK(thread_checker_.CalledOnValidThread());
   TabEntryMap::const_iterator tab_entry_iterator = active_tabs_.find(tab_id);
@@ skipping 164 matching lines @@
     DCHECK(oldest_tab_entry_iterator != active_tabs_.end());
     UMA_HISTOGRAM_CUSTOM_TIMES(
         kUMAUnexpiredTabEntryRemovalDurationHistogram,
         NowTicks() -
             oldest_tab_entry_iterator->second.GetLatestStartOrEndTime(),
         base::TimeDelta::FromMinutes(1), base::TimeDelta::FromHours(1), 50);
     active_tabs_.erase(oldest_tab_entry_iterator);
   }
 }
 
+void DataUseTabModel::ProcessBufferedNavigationEvents() {
+  if (!ui_navigation_buffer)

[tbansal1, 2016/03/08 18:15:00]

DCHECK (thread_checker_...)

+    return;
+  scoped_ptr<std::vector<DataUseUINavigationBuffer>> tmp_ui_navigation_buffer =

[tbansal1, 2016/03/08 18:15:00]

Why move it to a tmp vector? Can we not iterate over the original vector, and
then empty/delete it afterwards?

+      std::move(ui_navigation_buffer);
+  for (const auto& ui_event : *tmp_ui_navigation_buffer.get()) {
+    OnNavigationEvent(ui_event.tab_id, ui_event.transition, ui_event.url,
+                      ui_event.package);
+  }
+}
+
 }  // namespace android
 
 }  // namespace chrome