Support needed to measure user and service traffic in Chrome.

chrome/browser/net/chrome_network_delegate_unittest.cc

Index: chrome/browser/net/chrome_network_delegate_unittest.cc
diff --git a/chrome/browser/net/chrome_network_delegate_unittest.cc b/chrome/browser/net/chrome_network_delegate_unittest.cc
index 3ca7bbe09a078acaab591f43fdcdc47c065fa020..4448b090f04b3fc8e9024c45611e8145a4527aa6 100644
--- a/chrome/browser/net/chrome_network_delegate_unittest.cc
+++ b/chrome/browser/net/chrome_network_delegate_unittest.cc
@@ -9,6 +9,8 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_member.h"
+#include "base/run_loop.h"
+#include "base/test/histogram_tester.h"
 #include "chrome/browser/content_settings/cookie_settings_factory.h"
 #include "chrome/browser/net/safe_search_util.h"
 #include "chrome/common/pref_names.h"
@@ -16,45 +18,239 @@
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/common/pref_names.h"
+#include "content/public/browser/resource_request_info.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/common/resource_type.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/base/request_priority.h"
+#include "net/socket/socket_test_util.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_test_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/event_router_forwarder.h"
 #endif
 
-TEST(ChromeNetworkDelegateTest, DisableFirstPartyOnlyCookiesIffFlagDisabled) {
-  BooleanPrefMember pref_member_;
-  scoped_ptr<ChromeNetworkDelegate> delegate;
+#if !defined(OS_IOS)
+#include "components/data_use_measurement/core/data_use_user_data.h"
+#endif
+
+namespace {
+
+// This function requests a URL. If |from_user| is true, it attaches a

[mmenke, 2015/09/04 15:52:00]

Maybe "This function requests a URL." -> "This function requests a URL, and
makes it return a known response."

[amohammadkhan, 2015/09/04 17:25:11]

Done.

+// ResourceRequestInfo to the URLRequest, because requests from users have
+// this info. If |from_user| is false, the request is presumed to be from a
+// service, and the service name is set in the request's user data. (As an
+// example suggestions service tag is attached). if |redirect| is true, it adds
+// necessary socket data to have a redirection before the main request.

[mmenke, 2015/09/04 15:52:01]

"necessary socket data to have it follow redirect before getting the final
response."

[amohammadkhan, 2015/09/04 17:25:11]

Done.

+void RequestURL(const net::URLRequestContext* context,

[mmenke, 2015/09/04 15:52:00]

remove const.

[amohammadkhan, 2015/09/04 17:25:12]

Done.

+                net::MockClientSocketFactory* socket_factory,
+                bool from_user,
+                bool redirect) {
+  net::MockRead redirect_mock_reads[] = {
+      net::MockRead("HTTP/1.1 302 Found\r\n"
+                    "Location: http://bar.com/\r\n\r\n"),
+      net::MockRead(""), net::MockRead(net::SYNCHRONOUS, net::OK),

[mmenke, 2015/09/04 15:52:01]

nit:  Here, and below, suggest one read per line.

[amohammadkhan, 2015/09/04 17:25:12]

Done. Although "git cl format" makes it in that form and I had it in separate
lines.

[mmenke, 2015/09/04 20:51:45]

Ahh...I'm happy to defer to git cl format, then.

+  };
+  net::StaticSocketDataProvider redirect_socket_data_provider(
+      redirect_mock_reads, arraysize(redirect_mock_reads), nullptr, 0);
+  if (redirect) {
+    socket_factory->AddSocketDataProvider(&redirect_socket_data_provider);
+  }
 
+  net::MockRead response_mock_reads[] = {
+      net::MockRead("HTTP/1.1 200 OK\r\n\r\n"), net::MockRead("response body"),
+      net::MockRead(net::SYNCHRONOUS, net::OK),
+  };
+  net::StaticSocketDataProvider response_socket_data_provider(
+      response_mock_reads, arraysize(response_mock_reads), nullptr, 0);
+  socket_factory->AddSocketDataProvider(&response_socket_data_provider);
+
+  net::TestDelegate test_delegate;
+  test_delegate.set_quit_on_complete(true);
+
+  scoped_ptr<net::URLRequest> request(context->CreateRequest(
+      GURL("http://example.com"), net::DEFAULT_PRIORITY, &test_delegate));
+  if (from_user) {
+    content::ResourceRequestInfo::AllocateForTesting(
+        request.get(), content::RESOURCE_TYPE_MAIN_FRAME, nullptr, -2, -2, -2,
+        true, false, true, true);
+  } else {
+    request->SetUserData(
+        data_use_measurement::DataUseUserData::kUserDataKey,
+        new data_use_measurement::DataUseUserData(
+            data_use_measurement::DataUseUserData::SUGGESTIONS));
+  }
+  request->Start();
+  base::RunLoop().RunUntilIdle();
+}
+
+}  // namespace
+
+class ChromeNetworkDelegateTest : public testing::Test {
+ public:
+  ChromeNetworkDelegateTest()
+      : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
+        context_(true) {
 #if defined(ENABLE_EXTENSIONS)
-  scoped_refptr<extensions::EventRouterForwarder> forwarder =
-      new extensions::EventRouterForwarder();
-  delegate.reset(new ChromeNetworkDelegate(forwarder.get(), &pref_member_));
-#else
-  delegate.reset(new ChromeNetworkDelegate(nullptr, &pref_member_));
+    forwarder_ = new extensions::EventRouterForwarder();
 #endif
-  EXPECT_FALSE(delegate->FirstPartyOnlyCookieExperimentEnabled());
-}
+  }
 
-TEST(ChromeNetworkDelegateTest, EnableFirstPartyOnlyCookiesIffFlagEnabled) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableExperimentalWebPlatformFeatures);
-  BooleanPrefMember pref_member_;
-  scoped_ptr<ChromeNetworkDelegate> delegate;
+  void SetUp() override {
+    ChromeNetworkDelegate::InitializePrefsOnUIThread(
+        &enable_referrers_, nullptr, nullptr, nullptr,
+        profile_.GetTestingPrefService());
+  }
+
+ protected:

[mmenke, 2015/09/04 15:52:00]

nit:  Protected doesn't really get us anything in tests.

[amohammadkhan, 2015/09/04 17:25:11]

Done.

+  scoped_ptr<net::NetworkDelegate> CreateNetworkDelegate() {
+    scoped_ptr<ChromeNetworkDelegate> network_delegate(
+        new ChromeNetworkDelegate(forwarder(), &enable_referrers_));
+    return network_delegate.Pass();
+  }

[mmenke, 2015/09/04 15:52:01]

You are correct.  I think it's cleaner to just do:

void Initialize() {
  // Create context and network delegate here
  // (Switch to using a scoped_ptr for both in the test fixture).
}

(Or call it CreateNetworkDelegateAndContext or something - happy with other
ideas on the name)

[amohammadkhan, 2015/09/04 17:25:11]

Done.

+
+  void SetDelegate(net::NetworkDelegate* delegate) {
+    network_delegate_ = delegate;
+    context_.set_client_socket_factory(&socket_factory_);
+    context_.set_network_delegate(network_delegate_);
+    context_.Init();
+  }
 
+  net::TestURLRequestContext* context() { return &context_; }
+  net::NetworkDelegate* network_delegate() { return network_delegate_; }
+  net::MockClientSocketFactory* socket_factory() { return &socket_factory_; }
+  extensions::EventRouterForwarder* forwarder() {

[mmenke, 2015/09/04 15:52:01]

Suggest a blank line above forwarder, to make the code easier to read.

[amohammadkhan, 2015/09/04 17:25:11]

Done.

 #if defined(ENABLE_EXTENSIONS)
-  scoped_refptr<extensions::EventRouterForwarder> forwarder =
-      new extensions::EventRouterForwarder();
-  delegate.reset(new ChromeNetworkDelegate(forwarder.get(), &pref_member_));
+    return forwarder_.get();
 #else
-  delegate.reset(new ChromeNetworkDelegate(nullptr, &pref_member_));
+    return nullptr;
+#endif
+  }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+  net::TestURLRequestContext context_;
+  net::NetworkDelegate* network_delegate_;

[mmenke, 2015/09/04 15:52:01]

This should go before the context_, as the context depends on it.

[amohammadkhan, 2015/09/04 17:25:11]

Done.

+  net::MockClientSocketFactory socket_factory_;
+#if defined(ENABLE_EXTENSIONS)
+  scoped_refptr<extensions::EventRouterForwarder> forwarder_;
+#endif
+  TestingProfile profile_;
+  BooleanPrefMember enable_referrers_;

[mmenke, 2015/09/04 15:52:00]

These last three should all go before the network delegate, as it depends on
them.

[amohammadkhan, 2015/09/04 17:25:11]

Done.

+};
+
+// This function tests data use measurement for requests by services. it makes a
+// query which is similar to a query of a service, so it should affect
+// DataUse.TrafficSize.System.Dimensions and DataUse.MessageSize.ServiceName
+// histograms. AppState and ConnectionType dimensions are always Foreground and
+// NotCellular respectively.
+#if !defined(OS_IOS)
+TEST_F(ChromeNetworkDelegateTest, DataUseMeasurementServiceTest) {
+  scoped_ptr<net::NetworkDelegate> delegate(CreateNetworkDelegate());
+  SetDelegate(delegate.get());
+  base::HistogramTester histogram_tester;
+
+  // A query from a service without redirection.
+  RequestURL(context(), socket_factory(), 0, 0);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.System.Downstream.Foreground.NotCellular", 1);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.System.Upstream.Foreground.NotCellular", 1);
+  // One upload and one download message, so totalCount should be 2.
+  histogram_tester.ExpectTotalCount("DataUse.MessageSize.Suggestions", 2);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.User.Downstream.Foreground.NotCellular", 0);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.User.Upstream.Foreground.NotCellular", 0);
+}
+
+// This function tests data use measurement for requests by user.The query from
+// a user should affect DataUse.TrafficSize.User.Dimensions histogram. AppState
+// and ConnectionType dimensions are always Foreground and NotCellular
+// respectively.
+TEST_F(ChromeNetworkDelegateTest, DataUseMeasurementUserTest) {
+  scoped_ptr<net::NetworkDelegate> delegate(CreateNetworkDelegate());
+  SetDelegate(delegate.get());
+  base::HistogramTester histogram_tester;
+
+  // A query from user without redirection.
+  RequestURL(context(), socket_factory(), 1, 0);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.User.Downstream.Foreground.NotCellular", 1);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.User.Upstream.Foreground.NotCellular", 1);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.System.Downstream.Foreground.NotCellular", 0);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.System.Upstream.Foreground.NotCellular", 0);
+  histogram_tester.ExpectTotalCount("DataUse.MessageSize.Suggestions", 0);
+}
+
+// This function tests data use measurement for requests by services in case the
+// request is redirected once. it makes a query which is similar to a query of a
+// service, so it should affect DataUse.TrafficSize.System.Dimensions and
+// DataUse.MessageSize.ServiceName histograms. AppState and ConnectionType
+// dimensions are always Foreground and NotCellular respectively.
+TEST_F(ChromeNetworkDelegateTest,
+       DataUseMeasurementServiceTestWithRedirecting) {

[mmenke, 2015/09/04 15:52:01]

nit:  Redirecting->Redirect

[amohammadkhan, 2015/09/04 17:25:11]

Done.

+  scoped_ptr<net::NetworkDelegate> delegate(CreateNetworkDelegate());
+  SetDelegate(delegate.get());
+  base::HistogramTester histogram_tester;
+
+  // A query from user with one redirection.
+  RequestURL(context(), socket_factory(), 0, 1);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.System.Downstream.Foreground.NotCellular", 2);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.System.Upstream.Foreground.NotCellular", 2);
+  // One upload and one download message, so totalCount should be 2.
+  histogram_tester.ExpectTotalCount("DataUse.MessageSize.Suggestions", 4);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.User.Downstream.Foreground.NotCellular", 0);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.User.Upstream.Foreground.NotCellular", 0);
+}
+
+// This function tests data use measurement for requests by user in case the
+// request is redirected once.The query from a user should affect
+// DataUse.TrafficSize.User.Dimensions histogram. AppState and ConnectionType
+// dimensions are always Foreground and NotCellular respectively.
+TEST_F(ChromeNetworkDelegateTest, DataUseMeasurementUserTestWithRedirecting) {
+  scoped_ptr<net::NetworkDelegate> delegate(CreateNetworkDelegate());
+  SetDelegate(delegate.get());
+  base::HistogramTester histogram_tester;
+
+  // A query from user with one redirection.
+  RequestURL(context(), socket_factory(), 1, 1);
+
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.User.Downstream.Foreground.NotCellular", 2);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.User.Upstream.Foreground.NotCellular", 2);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.System.Downstream.Foreground.NotCellular", 0);
+  histogram_tester.ExpectTotalCount(
+      "DataUse.TrafficSize.System.Upstream.Foreground.NotCellular", 0);
+  histogram_tester.ExpectTotalCount("DataUse.MessageSize.Suggestions", 0);
+}
+
 #endif
-  EXPECT_TRUE(delegate->FirstPartyOnlyCookieExperimentEnabled());
+
+TEST_F(ChromeNetworkDelegateTest, DisableFirstPartyOnlyCookiesIffFlagDisabled) {
+  scoped_ptr<net::NetworkDelegate> delegate(CreateNetworkDelegate());
+  SetDelegate(delegate.get());
+  EXPECT_FALSE(network_delegate()->FirstPartyOnlyCookieExperimentEnabled());
+}
+
+TEST_F(ChromeNetworkDelegateTest, EnableFirstPartyOnlyCookiesIffFlagEnabled) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableExperimentalWebPlatformFeatures);
+  scoped_ptr<net::NetworkDelegate> delegate(CreateNetworkDelegate());
+  SetDelegate(delegate.get());
+  EXPECT_TRUE(network_delegate()->FirstPartyOnlyCookieExperimentEnabled());
 }
 
 class ChromeNetworkDelegateSafeSearchTest : public testing::Test {