Add external data use observer

chrome/browser/android/datausage/external_data_use_observer_android.h

Index: chrome/browser/android/datausage/external_data_use_observer_android.h
diff --git a/chrome/browser/android/datausage/external_data_use_observer_android.h b/chrome/browser/android/datausage/external_data_use_observer_android.h
new file mode 100644
index 0000000000000000000000000000000000000000..3eb74c9c8169e6dc8b550e86942c58669f588f3a
--- /dev/null
+++ b/chrome/browser/android/datausage/external_data_use_observer_android.h
@@ -0,0 +1,79 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.

[newt (away), 2015/10/12 18:01:18]

Since this class is already inside an "android" folder and since there's no
equivalent of this class for other platforms besides Android, it doesn't need
the "_android" suffix at the end of the filename or class name.

[tbansal1, 2015/10/12 22:39:14]

In the past, I did this and later had to add a base class which required
too much code churn and renaming of classes. Is it okay if I keep the
"android" suffix? or, is it totally against the conventions?

[newt (away), 2015/10/13 19:05:20]

If it seems likely that you'll need to add a base class or platform-independent
class, then it's OK to use the "_android" suffix, but otherwise I'd avoid it.

[tbansal1, 2015/10/13 21:30:20]

OK, for now I don't forecast any platform independent class. So, removing
the suffix.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_ANDROID_DATAUSAGE_EXTERNAL_DATA_USE_OBSERVER_ANDROID_H_
+#define CHROME_BROWSER_ANDROID_DATAUSAGE_EXTERNAL_DATA_USE_OBSERVER_ANDROID_H_
+
+#include <jni.h>
+#include <string>
+#include <vector>
+
+#include "base/android/scoped_java_ref.h"
+#include "base/gtest_prod_util.h"
+#include "base/macros.h"
+#include "base/threading/thread_checker.h"
+#include "cc/base/scoped_ptr_vector.h"

[sclittle, 2015/10/12 20:10:15]

Why "cc/base/scoped_ptr_vector.h" instead of "base/memory/scoped_vector.h"?

[tbansal1, 2015/10/12 22:39:14]

scoped_vector requires creating raw pointers and then pushing
them on the ScopedVector object.

However, if I create raw pointers, and later it turns out that the pattern is
invalid,
then I can't push it on the ScopedVector and have to manually delete the raw
pointer.

I think ScopedPtrVector better suits our purpose since scoped_ptr will
automatically delete.

[sclittle, 2015/10/13 19:46:56]

ScopedVectors can also take scoped_ptrs:
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/scoped...

I'd recommend using base/memory/scoped_vector.h over cc/base/scoped_ptr_vector.h
- I've never seen this "cc/" folder used, and I don't know what is allowed to
depend on it, or if it exists on all platforms.

[tbansal1, 2015/10/13 21:30:20]

Done.

+#include "components/data_usage/core/data_use.h"
+#include "components/data_usage/core/data_use_aggregator.h"
+
+namespace re2 {
+class RE2;
+}
+
+namespace chrome {
+
+namespace android {
+
+// This class defines a data use observer external to Chromium. It creates and

[newt (away), 2015/10/12 18:01:18]

This comment should be much clearer. Imagine that you're explaining this
class/feature to someone who hasn't read the design doc, and give an overview of
what this class does / why it exists, before you dive into implementation
details.

In particular, after reading this comment, I have several (major) questions:
 - What does "external to Chromium" mean? Is this a way that other apps can
observe Chromium's data usage, or that Chromium can observe other app's data
usage? This is a critical distinction.
 - What are these regular expressions used for? Are they matched against app
names, or app IDs, or web address, or what? If the use of these regexes is
already explained in DataUseAggregator, then you could just say, "see
DataUseAggregator::FooRegexMethod()".

Here's how the comment might read: "This class allows C++ classes to observe how
much data is used by other apps (besides Chromium) on the current Android
device. It accepts a regex that allows for filtering data usage by URLs that
match the regex. ... etc"

[tbansal1, 2015/10/12 22:39:14]

Added more documentation.

+// owns a Java listener object that listens to data use observations. Native
+// code receives regular expressions from the Java listener. This class also
+// registers as a data use observer with DataUseAggregator, applies the regular
+// expressions to the received observations, and reports the matching data use
+// reports to the Java listener.
+class ExternalDataUseObserverAndroid
+    : public data_usage::DataUseAggregator::Observer {
+ public:
+  explicit ExternalDataUseObserverAndroid(
+      data_usage::DataUseAggregator* data_use_aggregator);
+  ~ExternalDataUseObserverAndroid() override;
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverAndroidTest, SingleRegex);
+  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverAndroidTest, TwoRegex);
+  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverAndroidTest, MultipleRegex);
+  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverAndroidTest, ChangeRegex);
+
+  // data_usage::DataUseAggregator::Observer implementation:
+  void OnDataUse(
+      const std::vector<data_usage::DataUse>& data_use_sequence) override;
+
+  // Called by Java to register multiple regular expressions. If the url of the
+  // data use request matches any of the regular expression, the observation is
+  // passed to the Java listener.
+  void RegisterURLRegexes(const std::vector<std::string>& url_regexes);
+
+  // Returns true if the |gurl| matches the registered regular expressions.
+  bool Matches(const GURL& gurl) const;
+
+  // Aggregator that sends data use observations.
+  data_usage::DataUseAggregator* data_use_aggregator_;
+
+  // Java listener.
+  base::android::ScopedJavaGlobalRef<jobject> j_external_data_use_observer_;
+
+  // Registered RE2 patterns that are matched against the URLs.
+  cc::ScopedPtrVector<re2::RE2> url_patterns_;
+
+  base::ThreadChecker thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExternalDataUseObserverAndroid);
+};
+
+bool RegisterExternalDataUseObserverAndroid(JNIEnv* env);
+
+}  // namespace android
+
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_ANDROID_DATAUSAGE_EXTERNAL_DATA_USE_OBSERVER_ANDROID_H_