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

net/url_request/url_request_file_job_unittest.cc

 // Copyright 2014 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 "net/url_request/url_request_file_job.h"
 
 #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,
                           network_delegate,
                           file_path,
                           file_task_runner),
         seek_position_(0) {
   }
 
   int64 seek_position() { return seek_position_; }
   const std::vector<std::string>& data_chunks() { return data_chunks_; }
 
  protected:
   virtual ~URLRequestFileJobWithCallbacks() {}
 
   virtual void OnSeekComplete(int64 result) OVERRIDE {
     ASSERT_EQ(seek_position_, 0);
     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));
   }
 
   int64 seek_position_;
   std::vector<std::string> data_chunks_;
 };
 
 // A URLRequestJobFactory that will return URLRequestFileJobWithCallbacks
 // instances for file:// scheme URLs.
-class CallbacksJobFactory : public net::URLRequestJobFactory {
+class CallbacksJobFactory : public URLRequestJobFactory {
  public:
   class JobObserver {
    public:
     virtual void OnJobCreated(URLRequestFileJobWithCallbacks* job) = 0;
   };
 
   CallbacksJobFactory(const base::FilePath& path, JobObserver* observer)
       : path_(path), observer_(observer) {
   }
 
   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,
         path_,
         base::MessageLoop::current()->message_loop_proxy());
     observer_->OnJobCreated(job);
     return job;
   }
 
   virtual bool IsHandledProtocol(const std::string& scheme) const OVERRIDE {
@@ skipping 67 matching lines @@
  protected:
   // This creates a file with |content| as the contents, and then creates and
   // runs a URLRequestFileJobWithCallbacks job to get the contents out of it,
   // and makes sure that the callbacks observed the correct bytes. If a Range
   // is provided, this function will add the appropriate Range http header to
   // the request and verify that only the bytes in that range (inclusive) were
   // observed.
   void RunRequest(const std::string& content, const Range* range);
 
   JobObserverImpl observer_;
-  net::TestURLRequestContext context_;
-  net::TestDelegate delegate_;
+  TestURLRequestContext context_;
+  TestDelegate delegate_;
 };
 
 URLRequestFileJobEventsTest::URLRequestFileJobEventsTest() {}
 
 void URLRequestFileJobEventsTest::RunRequest(const std::string& content,
                                              const Range* range) {
   base::ScopedTempDir directory;
   ASSERT_TRUE(directory.CreateUniqueTempDir());
   base::FilePath path;
   ASSERT_TRUE(CreateTempFileWithContent(content, directory, &path));
   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);
     ASSERT_LE(range->start, range->end);
     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();
 
   EXPECT_FALSE(delegate_.request_failed());
   int expected_length =
       range ? (range->end - range->start + 1) : content.length();
   EXPECT_EQ(delegate_.bytes_received(), expected_length);
 
   std::string expected_content;
   if (range) {
     expected_content.insert(0, content, range->start, expected_length);
   } else {
     expected_content = content;
   }
   EXPECT_TRUE(delegate_.data_received() == expected_content);
 
   ASSERT_EQ(observer_.jobs().size(), 1u);
   ASSERT_EQ(observer_.jobs().at(0)->seek_position(), range ? range->start : 0);
 
   std::string observed_content;
   const std::vector<std::string>& chunks =
       observer_.jobs().at(0)->data_chunks();
   for (std::vector<std::string>::const_iterator i = chunks.begin();
        i != chunks.end();
        ++i) {
     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
 // test content.
 std::string MakeContentOfSize(int size) {
   EXPECT_GE(size, 0);
   std::string result;
   result.reserve(size);
   for (int i = 0; i < size; i++) {
     result.append(1, static_cast<char>(i % 256));
   }
   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);
 }
 
 TEST_F(URLRequestFileJobEventsTest, SmallFile) {
   RunRequest(MakeContentOfSize(17 * 1024), NULL);
 }
 
 TEST_F(URLRequestFileJobEventsTest, BigFile) {
   RunRequest(MakeContentOfSize(3 * 1024 * 1024), NULL);
 }
 
 TEST_F(URLRequestFileJobEventsTest, Range) {
   // Use a 15KB content file and read a range chosen somewhat arbitrarily but
   // not aligned on any likely page boundaries.
   int size = 15 * 1024;
   Range range(1701, (6 * 1024) + 3);
   RunRequest(MakeContentOfSize(size), &range);
 }
+
+}  // namespace
+
+}  // namespace net