Notify DataUseTabModel of navigations and tab closures

chrome/browser/android/data_usage/external_data_use_observer.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_EXTERNAL_DATA_USE_OBSERVER_H_
 #define CHROME_BROWSER_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_
 
 #include <jni.h>
 #include <stdint.h>
 
@@ skipping 94 matching lines @@
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, MultipleMatchingRules);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, ReportsMergedCorrectly);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest,
                            TimestampsMergedCorrectly);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, HashFunction);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, BufferSize);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest,
                            PeriodicFetchMatchingRules);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, BufferDataUseReports);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, Variations);
+  FRIEND_TEST_ALL_PREFIXES(DataUseUITabModelTest, ReportTabEventsTest);

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

Why is this necessary? Can we please not make this necessary?

If many tests might want to call RegisterURLRegexes, then you could add a public
RegisterURLRegexesForTesting method, which is checked by presubmits to ensure
that it's only called by test code.

At the very least, please add a TODO for this and alphabetize this list.

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

The public function requires Java object and Java environment setup.
I thought FRIEND_TEST was slightly better because it avoid adding methods that
are called only by tests (which may increase the size of binary). I am probably
wrong?
Done.

 
   // DataUseReportKey is a unique identifier for a data use report.
   struct DataUseReportKey {
     DataUseReportKey(const std::string& label,
                      net::NetworkChangeNotifier::ConnectionType connection_type,
                      const std::string& mcc_mnc);
 
     bool operator==(const DataUseReportKey& other) const;
 
     // Label provided by the matching rules.
@@ skipping 182 matching lines @@
   const base::TimeDelta fetch_matching_rules_duration_;
 
   // Minimum number of bytes that should be buffered before a data use report is
   // submitted.
   const int64_t data_use_report_min_bytes_;
 
   base::ThreadChecker thread_checker_;
 
   // |io_weak_factory_| and |ui_weak_factory_| are used for posting tasks on the
   // IO and UI thread, respectively.
   base::WeakPtrFactory<ExternalDataUseObserver> io_weak_factory_;

[sclittle, 2015/11/17 23:13:46]

For a future CL: WeakPtrs aren't safe to be invalidated or dereferenced on
different threads than the thread that the object will be destroyed on. Please
fix this.

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

Acknowledged. Yes, currently doing that in a separate CL.

   base::WeakPtrFactory<ExternalDataUseObserver> ui_weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ExternalDataUseObserver);
 };
 
 bool RegisterExternalDataUseObserver(JNIEnv* env);
 
 }  // namespace android
 
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_