Add tests for HttpRequestCompletionErrorCodes

chrome/browser/net/chrome_network_delegate_unittest.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/net/chrome_network_delegate.h"
 
 #include <stdint.h>
 
 #include <memory>
 #include <utility>
@@ skipping 32 matching lines @@
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/event_router_forwarder.h"
 #endif
 
 namespace {
 
+const char* const kHttpRequestCompletionErrorCodes =
+    "Net.HttpRequestCompletionErrorCodes";

[asanka, 2017/03/07 19:15:47]

Use something like:

const char kHttpRequestCompletionErrorCodes[] = "...";

... which accomplishes what you are doing with the extra 'const's without
introducing a pointer.


Also let's move this to the function that uses these constants. Keeping them
here would only be useful if multiple tests were using these constants.

[Amey J, 2017/03/08 02:05:18]

Acknowledged.

+const char* const kHttpRequestCompletionErrorCodesMainFrame =
+    "Net.HttpRequestCompletionErrorCodes.MainFrame";
+
 // This function requests a URL, and makes it return a known response.
 // ResourceRequestInfo is attached to the URLRequest, to represent this request
 // as an user initiated.
 std::unique_ptr<net::URLRequest> RequestURL(
     net::URLRequestContext* context,
     net::MockClientSocketFactory* socket_factory) {
   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::SYNCHRONOUS, net::OK),
@@ skipping 165 matching lines @@
       RequestURL(context(), socket_factory());
 
   EXPECT_EQ(0, fake_aggregator.on_the_record_tx_bytes());
   EXPECT_EQ(0, fake_aggregator.on_the_record_rx_bytes());
   EXPECT_EQ(request->GetTotalSentBytes(),
             fake_aggregator.off_the_record_tx_bytes());
   EXPECT_EQ(request->GetTotalReceivedBytes(),
             fake_aggregator.off_the_record_rx_bytes());
 }
 
+TEST_F(ChromeNetworkDelegateTest, HttpRequestCompletionErrorCodes) {
+  Initialize();
+
+  const struct {
+    const GURL url;
+    int net_error;
+    bool main_frame;

[asanka, 2017/03/07 19:15:47]

is_main_frame

[Amey J, 2017/03/08 02:05:18]

Acknowledged.

+    int expected_sample_bucket;
+    int expected_request_completion_count;
+    int expected_request_completion_main_frame_count;
+  } tests[] = {
+      {GURL("http://example.com"), net::OK, true, std::abs(net::OK), 1, 1},
+      {GURL("http://example.com"), net::ERR_ABORTED, true,
+       std::abs(net::ERR_ABORTED), 1, 1},
+      {GURL("http://example.com"), net::OK, false, std::abs(net::OK), 1, 0},
+      {GURL("https://example.com"), net::OK, true, std::abs(net::OK), 0, 0},
+  };
+
+  for (size_t i = 0; i < arraysize(tests); ++i) {

[asanka, 2017/03/07 19:15:47]

use a range based iterator. I.e. something like:

  for (const auto& test : tests) {
     ...
     // and use 'test' to refer to the test case instead of 'tests[i]'.

[Amey J, 2017/03/08 02:05:18]

Sweet! Somehow, I always thought that range based loops are just for
std::iterators.

+    base::HistogramTester histograms;
+
+    net::TestDelegate test_delegate;
+    std::unique_ptr<net::URLRequest> request(context()->CreateRequest(
+        tests[i].url, net::DEFAULT_PRIORITY, &test_delegate));
+    if (tests[i].main_frame) {
+      request->SetLoadFlags(request->load_flags() |
+                            net::LOAD_MAIN_FRAME_DEPRECATED);
+    }
+    network_delegate()->NotifyCompleted(request.get(), false,
+                                        tests[i].net_error);
+
+    histograms.ExpectTotalCount(kHttpRequestCompletionErrorCodes,
+                                tests[i].expected_request_completion_count);
+    histograms.ExpectUniqueSample(kHttpRequestCompletionErrorCodes,
+                                  tests[i].expected_sample_bucket,
+                                  tests[i].expected_request_completion_count);
+    histograms.ExpectTotalCount(
+        kHttpRequestCompletionErrorCodesMainFrame,
+        tests[i].expected_request_completion_main_frame_count);
+    histograms.ExpectUniqueSample(
+        kHttpRequestCompletionErrorCodesMainFrame,
+        tests[i].expected_sample_bucket,
+        tests[i].expected_request_completion_main_frame_count);
+  }
+}
+
 class ChromeNetworkDelegatePolicyTest : public testing::Test {
  public:
   ChromeNetworkDelegatePolicyTest()
       : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
     forwarder_ = new extensions::EventRouterForwarder();
 #endif
   }
 
  protected:
@@ skipping 335 matching lines @@
 
   // Files in external storage are allowed.
   base::FilePath external_storage_path;
   PathService::Get(base::DIR_ANDROID_EXTERNAL_STORAGE, &external_storage_path);
   EXPECT_TRUE(IsAccessAllowed(
       external_storage_path.AppendASCII("foo.txt").AsUTF8Unsafe(), ""));
   // The external storage root itself is not allowed.
   EXPECT_FALSE(IsAccessAllowed(external_storage_path.AsUTF8Unsafe(), ""));
 #endif
 }