Notify DataUseTabModel of navigations and tab closures

chrome/browser/android/data_usage/data_use_ui_tab_model.h

Index: chrome/browser/android/data_usage/data_use_ui_tab_model.h
diff --git a/chrome/browser/android/data_usage/data_use_ui_tab_model.h b/chrome/browser/android/data_usage/data_use_ui_tab_model.h
index a8dc492f521ad5e92f65f86ce1b2ddd2246189d0..103049e7b15941ae9e040e2c6748d0be0ebaab58 100644
--- a/chrome/browser/android/data_usage/data_use_ui_tab_model.h
+++ b/chrome/browser/android/data_usage/data_use_ui_tab_model.h
@@ -8,13 +8,15 @@
 #include <stdint.h>
 
 #include <string>
-#include <unordered_map>
 
+#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/weak_ptr.h"
+#include "base/single_thread_task_runner.h"
 #include "base/threading/thread_checker.h"
+#include "chrome/browser/android/data_usage/data_use_tab_model.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "ui/base/page_transition_types.h"
 
@@ -37,8 +39,8 @@ namespace android {
 // is removed from the list of tabs.
 class DataUseUITabModel : public KeyedService {
  public:
-  explicit DataUseUITabModel(
-      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner);
+  DataUseUITabModel(scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+                    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
   ~DataUseUITabModel() override;
 
   // Reports a browser navigation to the DataUseTabModel on IO thread. Includes
@@ -67,8 +69,72 @@ class DataUseUITabModel : public KeyedService {
   // Calling this function resets the state of the tab.
   bool HasDataUseTrackingEnded(int32_t tab_id);
 
+  // Sets the weak pointer to DataUseTabModel that can be used on IO thread.
+  void SetIODataUseTabModel(base::WeakPtr<DataUseTabModel> data_use_tab_model);

[sclittle, 2015/11/17 22:50:11]

This should be private.

[tbansal1, 2015/11/18 01:32:23]

Done.

[tbansal1, 2015/11/18 01:40:01]

This is called by profile_impl*, so can't be private.

+
+  // Notifies |this| of starting and ending of data tracking for tab with id
+  // |tab_id|.
+  void NotifyTrackingStarting(int32_t tab_id);
+  void NotifyTrackingEnding(int32_t tab_id);
+
+  // 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 TabObserverOnIOThread

[mmenke, 2015/11/17 22:48:40]

This doesn't need to be public, does it?  In fact, you can just forward declare
it as a private class in this file.

The methods in the anonymous namespace that much with this can be made into
static methods of DataUseUITabModel.

[sclittle, 2015/11/17 22:50:12]

Does this inner class definition need to be in the header file? Could you just
forward declare it here, and move all this class definition code to the .cc
file?

Also, this should be private, and no tests should have to access the Observer
directly.

[tbansal1, 2015/11/18 01:32:23]

Done.

[tbansal1, 2015/11/18 01:32:23]

Done.

+      : public base::RefCountedThreadSafe<TabObserverOnIOThread>,
+        public DataUseTabModel::TabDataUseObserver {
+   public:
+    TabObserverOnIOThread(
+        base::WeakPtr<DataUseUITabModel> data_use_ui_tab_model,
+        scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
+
+    // Sets the |data_use_tab_model_|, and registers as an observer to
+    // |data_use_tab_model_|.
+    void SetIODataUseTabModel(
+        base::WeakPtr<DataUseTabModel> data_use_tab_model);
+
+   private:
+    friend class DataUseUITabModelTest;
+
+    // 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_;
+
+    // True if |this| is registered as an observer to |data_use_tab_model_|.
+    bool registered_as_observer_;
+
+    base::ThreadChecker thread_checker_;
+
+    DISALLOW_COPY_AND_ASSIGN(TabObserverOnIOThread);
+  };
+
+  void SetTabDataUseObserver(
+      scoped_refptr<TabObserverOnIOThread> tab_data_use_observer);
+
  private:
+  // Friends for testing.
+  friend class DataUseUITabModelTest;
   FRIEND_TEST_ALL_PREFIXES(DataUseUITabModelTest, EntranceExitState);
+  FRIEND_TEST_ALL_PREFIXES(DataUseUITabModelTest, ConvertTransitionType);
+  FRIEND_TEST_ALL_PREFIXES(DataUseUITabModelTest, ReportTabEventsTest);
 
   // DataUseTrackingEvent indicates the state of a tab.
   enum DataUseTrackingEvent {
@@ -79,12 +145,7 @@ class DataUseUITabModel : public KeyedService {
     DATA_USE_TRACKING_ENDED,
   };
 
-  typedef std::unordered_map<int32_t, DataUseTrackingEvent> TabEvents;
-
-  // DataUseTabModel::Observer implementation:
-  // TODO(tbansal): Add override once DataUseTabModel is checked in.
-  void OnTrackingStarted(int32_t tab_id);
-  void OnTrackingEnded(int32_t tab_id);
+  typedef base::hash_map<int32_t, DataUseTrackingEvent> TabEvents;
 
   // Creates |event| for tab with id |tab_id| and value |event|, if there is no
   // existing entry for |tab_id|, and returns true. Otherwise, returns false
@@ -95,14 +156,33 @@ class DataUseUITabModel : public KeyedService {
   // returns true. Otherwise, returns false without modifying the entry.
   bool RemoveTabEvent(int32_t tab_id, DataUseTrackingEvent event);
 
+  // Converts |page_transition| to DataUseTabModel::TransitionType enum.
+  // Returns true if conversion was successful, and updates |transition_type|.
+  // Otherwise, returns false, and |transition_type| is not changed.
+  // |transition_type| must not be null.
+  bool ConvertTransitionType(
+      ui::PageTransition page_transition,
+      DataUseTabModel::TransitionType* transition_type) const;
+
+  // |tab_data_use_observer_| notifies |this| of starting and ending of data
+  // tracking.
+  scoped_refptr<TabObserverOnIOThread> tab_data_use_observer_;
+
   // |tab_events_| stores tracking events of multiple tabs.
   TabEvents tab_events_;
 
-  // |io_task_runner_| accesses DataUseTabModel members on IO thread.
+  // |io_data_use_tab_model_| is notified by |this| about browser navigations
+  // and tab closures on IO thread. |io_data_use_tab_model_| should only be
+  // used on IO thread.
+  base::WeakPtr<DataUseTabModel> io_data_use_tab_model_;
+
+  // Used to call methods on |tab_data_use_observer_|.
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
 
   base::ThreadChecker thread_checker_;
 
+  base::WeakPtrFactory<DataUseUITabModel> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(DataUseUITabModel);
 };