Make URLRequest's constructor private, and make URLRequestContext a friend class.

net/url_request/url_request_file_job_unittest.cc

Index: net/url_request/url_request_file_job_unittest.cc
diff --git a/net/url_request/url_request_file_job_unittest.cc b/net/url_request/url_request_file_job_unittest.cc
index 8e0cabb4bc33796e2b171d24b500e3bf81979c15..0163f13f92cc6a376bdfd5e569b525775935bfe6 100644
--- a/net/url_request/url_request_file_job_unittest.cc
+++ b/net/url_request/url_request_file_job_unittest.cc
@@ -6,22 +6,26 @@
 
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "net/base/filename_util.h"
 #include "net/base/net_util.h"
+#include "net/url_request/url_request.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace net {
+
 namespace {
 
 // A URLRequestFileJob for testing OnSeekComplete / OnReadComplete callbacks.
-class URLRequestFileJobWithCallbacks : public net::URLRequestFileJob {
+class URLRequestFileJobWithCallbacks : public URLRequestFileJob {
  public:
   URLRequestFileJobWithCallbacks(
-      net::URLRequest* request,
-      net::NetworkDelegate* network_delegate,
+      URLRequest* request,
+      NetworkDelegate* network_delegate,
       const base::FilePath& file_path,
       const scoped_refptr<base::TaskRunner>& file_task_runner)
       : URLRequestFileJob(request,
@@ -42,7 +46,7 @@ class URLRequestFileJobWithCallbacks : public net::URLRequestFileJob {
     seek_position_ = result;
   }
 
-  virtual void OnReadComplete(net::IOBuffer* buf, int result) OVERRIDE {
+  virtual void OnReadComplete(IOBuffer* buf, int result) OVERRIDE {
     data_chunks_.push_back(std::string(buf->data(), result));
   }
 
@@ -52,7 +56,7 @@ class URLRequestFileJobWithCallbacks : public net::URLRequestFileJob {
 
 // A URLRequestJobFactory that will return URLRequestFileJobWithCallbacks
 // instances for file:// scheme URLs.
-class CallbacksJobFactory : public net::URLRequestJobFactory {
+class CallbacksJobFactory : public URLRequestJobFactory {
  public:
   class JobObserver {
    public:
@@ -65,10 +69,10 @@ class CallbacksJobFactory : public net::URLRequestJobFactory {
 
   virtual ~CallbacksJobFactory() {}
 
-  virtual net::URLRequestJob* MaybeCreateJobWithProtocolHandler(
+  virtual URLRequestJob* MaybeCreateJobWithProtocolHandler(
       const std::string& scheme,
-      net::URLRequest* request,
-      net::NetworkDelegate* network_delegate) const OVERRIDE {
+      URLRequest* request,
+      NetworkDelegate* network_delegate) const OVERRIDE {
     URLRequestFileJobWithCallbacks* job = new URLRequestFileJobWithCallbacks(
         request,
         network_delegate,
@@ -156,8 +160,8 @@ class URLRequestFileJobEventsTest : public testing::Test {
   void RunRequest(const std::string& content, const Range* range);
 
   JobObserverImpl observer_;
-  net::TestURLRequestContext context_;
-  net::TestDelegate delegate_;
+  TestURLRequestContext context_;
+  TestDelegate delegate_;
 };
 
 URLRequestFileJobEventsTest::URLRequestFileJobEventsTest() {}
@@ -171,10 +175,8 @@ void URLRequestFileJobEventsTest::RunRequest(const std::string& content,
   CallbacksJobFactory factory(path, &observer_);
   context_.set_job_factory(&factory);
 
-  net::TestURLRequest request(net::FilePathToFileURL(path),
-                              net::DEFAULT_PRIORITY,
-                              &delegate_,
-                              &context_);
+  scoped_ptr<URLRequest> request(context_.CreateRequest(
+      FilePathToFileURL(path), DEFAULT_PRIORITY, &delegate_, NULL));
   if (range) {
     ASSERT_GE(range->start, 0);
     ASSERT_GE(range->end, 0);
@@ -182,10 +184,10 @@ void URLRequestFileJobEventsTest::RunRequest(const std::string& content,
     ASSERT_LT(static_cast<unsigned int>(range->end), content.length());
     std::string range_value =
         base::StringPrintf("bytes=%d-%d", range->start, range->end);
-    request.SetExtraRequestHeaderByName(
-        net::HttpRequestHeaders::kRange, range_value, true /*overwrite*/);
+    request->SetExtraRequestHeaderByName(
+        HttpRequestHeaders::kRange, range_value, true /*overwrite*/);
   }
-  request.Start();
+  request->Start();
 
   base::RunLoop loop;
   loop.Run();
@@ -215,7 +217,6 @@ void URLRequestFileJobEventsTest::RunRequest(const std::string& content,
     observed_content.append(*i);
   }
   EXPECT_EQ(expected_content, observed_content);
-  EXPECT_TRUE(expected_content == observed_content);
 }
 
 // Helper function to make a character array filled with |size| bytes of
@@ -230,8 +231,6 @@ std::string MakeContentOfSize(int size) {
   return result;
 }
 
-}  // namespace

[pauljensen, 2014/08/28 15:48:39]

nit: I liked this up here as I find it a nice divider between file-scoped and
globally-scoped/accessed-elsewhere items.  I guess the TEST_F() things aren't
really scoped but they are called from outside this file.  This is pretty much a
personal preference.  The style guide doesn't say much about this.  I'm fine
with the way you have it now.

[mmenke, 2014/08/28 16:46:00]

Each TEST_F does technically create a new class, which is globally scoped, and
could theoretically have name conflicts, I believe.

I generally go with the whole-file-in-abstract-namespace approach, because it
theoretically keeps stuff out of the global namespace, and it's been suggested
to me a couple times in previous codereviews.  But I really don't feel strongly
about this - I treat it like using braces in single line ifs - I may mention it
on codereviews, or change it, but often ignore it as well.

-
 TEST_F(URLRequestFileJobEventsTest, TinyFile) {
   RunRequest(std::string("hello world"), NULL);
 }
@@ -251,3 +250,7 @@ TEST_F(URLRequestFileJobEventsTest, Range) {
   Range range(1701, (6 * 1024) + 3);
   RunRequest(MakeContentOfSize(size), &range);
 }
+
+}  // namespace
+
+}  // namespace net