Remove one thread hop while fetching matching rules

chrome/browser/android/data_usage/data_use_tab_model.h

 // 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.
 
 #ifndef CHROME_BROWSER_ANDROID_DATA_USAGE_DATA_USE_TAB_MODEL_H_
 #define CHROME_BROWSER_ANDROID_DATA_USAGE_DATA_USE_TAB_MODEL_H_
 
 #include <stddef.h>
 
 #include <string>
 #include <vector>
 
 #include "base/containers/hash_tables.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 #include "chrome/browser/android/data_usage/tab_data_use_entry.h"
 #include "components/sessions/core/session_id.h"
+#include "url/gurl.h"
 
 namespace base {
 class SingleThreadTaskRunner;
 class TickClock;
 }
 
-class GURL;
-
 namespace chrome {
 
 namespace android {
 
 class DataUseMatcher;
-class ExternalDataUseObserver;
+class ExternalDataUseObserverBridge;
 
 // Models tracking and labeling of data usage within each Tab. Within each tab,
 // the model tracks the data use of a sequence of navigations in a "tracking
 // session" beginning with an entry event and ending with an exit event.
 // Typically, these events are navigations matching a URL pattern, or various
 // types of browser-initiated navigations. A single tab may have several
 // disjoint "tracking sessions" depending on the sequence of entry and exit
 // events that took place.
 class DataUseTabModel {
  public:
@@ skipping 30 matching lines @@
 
     // Notification callback when tab tracking sessions are started and ended.
     // The callback will be received on the same thread AddObserver was called
     // from.
     virtual void NotifyTrackingStarting(SessionID::id_type tab_id) = 0;
     virtual void NotifyTrackingEnding(SessionID::id_type tab_id) = 0;
   };
 
   DataUseTabModel();
 
-  // Initializes |this| on UI thread. |external_data_use_observer| is the weak
-  // pointer to ExternalDataUseObserver object that owns |this|.
+  // Initializes |this| on UI thread. |external_data_use_observer_bridge| is the
+  // pointer to ExternalDataUseObserverBridge object.

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

say more about ownership and lifetime: It seems the caller owns the bridge
object, but I am not sure if the bridge can get destroyed before |this|?

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

Done.

   void InitOnUIThread(
-      const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
-      const base::WeakPtr<ExternalDataUseObserver>& external_data_use_observer);
+      const ExternalDataUseObserverBridge* external_data_use_observer_bridge);
 
   virtual ~DataUseTabModel();
 
   base::WeakPtr<DataUseTabModel> GetWeakPtr();
 
   // Notifies the DataUseTabModel of navigation events. |tab_id| is the source
   // tab of the generated event, |transition| indicates the type of the UI
   // event/transition,  |url| is the URL in the source tab, |package| indicates
   // the android package name of external application that initiated the event.
   void OnNavigationEvent(SessionID::id_type tab_id,
@@ skipping 86 matching lines @@
   FRIEND_TEST_ALL_PREFIXES(DataUseTabModelTest, ObserverStartEndEvents);
   FRIEND_TEST_ALL_PREFIXES(DataUseTabModelTest, TabCloseEvent);
   FRIEND_TEST_ALL_PREFIXES(DataUseTabModelTest, TabCloseEventEndsTracking);
   FRIEND_TEST_ALL_PREFIXES(DataUseTabModelTest,
                            UnexpiredTabEntryRemovaltimeHistogram);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest,
                            MatchingRuleFetchOnControlAppInstall);
 
   typedef base::hash_map<SessionID::id_type, TabDataUseEntry> TabEntryMap;
 
+  // Contains the details of a single UI navigation event.
+  struct DataUseUINavigationBuffer {

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

This name seems incorrect. This is not a buffer/collection, more like a single
event.

+    SessionID::id_type tab_id;
+    TransitionType transition;

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

s/transition/transition_type/

+    GURL url;
+    std::string package;

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

nit: may be add comments for the fields. e.g., is the package name of the
package that started this navigation?

+  };
+
   // Gets the current label of a tab, and the new label if a navigation event
   // occurs in the tab. |tab_id| is the source tab of the generated event,
   // |transition| indicates the type of the UI event/transition,  |url| is the
   // URL in the source tab, |package| indicates the android package name of
   // external application that initiated the event. |current_label|, |new_label|
   // and |is_package_match| should not be null, and are set with current and new
   // labels respectively. |current_label| will be set to empty string, if there
   // is no active tracking session. |new_label| will be set to empty string if
   // there would be no active tracking session if the navigation happens.
   // |is_package_match| will be set to true if a tracking session will start due
@@ skipping 15 matching lines @@
 
   // Ends the current tracking session for tab with id |tab_id|.
   void EndTrackingDataUse(SessionID::id_type tab_id);
 
   // Compacts the tab entry map |active_tabs_| by removing expired tab entries.
   // After removing expired tab entries, if the size of |active_tabs_| exceeds
   // |kMaxTabEntries|, oldest unexpired tab entries will be removed until its
   // size is |kMaxTabEntries|.
   void CompactTabEntries();
 
+  void ProcessBufferedNavigationEvents();

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

function comments.

+
   // Collection of observers that receive tracking session start and end
   // notifications. Notifications are posted on UI thread.
   base::ObserverList<TabDataUseObserver> observers_;
 
   // Maintains the tracking sessions of multiple tabs.
   TabEntryMap active_tabs_;
 
   // Maximum number of tab entries to maintain session information about.
   const size_t max_tab_entries_;
 
   // Maximum number of tracking sessions to maintain per tab.
   const size_t max_sessions_per_tab_;
 
   // Expiration duration for a closed tab entry and an open tab entry
   // respectively.
   const base::TimeDelta closed_tab_expiration_duration_;
   const base::TimeDelta open_tab_expiration_duration_;
 
   // TickClock used for obtaining the current time.
   scoped_ptr<base::TickClock> tick_clock_;
 
   // Stores the matching patterns.
   scoped_ptr<DataUseMatcher> data_use_matcher_;
 
   // True if the external control app is installed.
   bool is_control_app_installed_;
 
+  // Buffers the list of UI navigation events that occurred until the first rule
+  // fetch is complete or the control app not installed callback was received or
+  // until |kDefaultMaxNavigationEventsBuffered| navigation events were
+  // buffered.
+  scoped_ptr<std::vector<DataUseUINavigationBuffer>> ui_navigation_buffer;

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

s/ui_navigation_buffer/data_use_ui_navigations_/?

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

Also, why does this need to be a scoped_ptr, and not a regular variable
std::vector<...> ui_navigation_buffer_;

May be add another variable
bool buffer_navigations_
instead of overriding this variable.

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

variable name should end with underscore.

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

say more about this buffer: what is the order in which events are stored. It
seems they are stored in FIFO?

+
   base::ThreadChecker thread_checker_;
 
   base::WeakPtrFactory<DataUseTabModel> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DataUseTabModel);
 };
 
 }  // namespace android
 
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_ANDROID_DATA_USAGE_DATA_USE_TAB_MODEL_H_