Make URLRequestJob::SetStatus private.

net/url_request/url_request_http_job_unittest.cc

Index: net/url_request/url_request_http_job_unittest.cc
diff --git a/net/url_request/url_request_http_job_unittest.cc b/net/url_request/url_request_http_job_unittest.cc
index 713ae608e40a4247fe371c6903a28b595dd29784..213e2d7a21859a7c1fb3d78a75da93aecb45503d 100644
--- a/net/url_request/url_request_http_job_unittest.cc
+++ b/net/url_request/url_request_http_job_unittest.cc
@@ -22,12 +22,14 @@
 #include "net/socket/socket_test_util.h"
 #include "net/test/cert_test_util.h"
 #include "net/url_request/url_request.h"
+#include "net/url_request/url_request_job_factory_impl.h"
 #include "net/url_request/url_request_status.h"
 #include "net/url_request/url_request_test_util.h"
 #include "net/websockets/websocket_handshake_stream_base.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
+#include "url/url_constants.h"
 
 namespace net {
 
@@ -52,11 +54,19 @@ class TestURLRequestHttpJob : public URLRequestHttpJob {
 
 class URLRequestHttpJobTest : public ::testing::Test {
  protected:
-  URLRequestHttpJobTest()
-      : req_(context_.CreateRequest(GURL("http://www.example.com"),
-                                    DEFAULT_PRIORITY,
-                                    &delegate_)) {
+  URLRequestHttpJobTest() : context_(true) {
     context_.set_http_transaction_factory(&network_layer_);
+
+    // The |test_job_factory_| takes ownership of the interceptor.
+    test_job_interceptor_ = new TestJobInterceptor();
+    EXPECT_TRUE(test_job_factory_.SetProtocolHandler(
+        url::kHttpScheme, make_scoped_ptr(test_job_interceptor_)));
+    context_.set_job_factory(&test_job_factory_);
+
+    context_.Init();
+
+    req_ = context_.CreateRequest(GURL("http://www.example.com"),
+                                  DEFAULT_PRIORITY, &delegate_);
   }
 
   bool TransactionAcceptsSdchEncoding() {
@@ -91,6 +101,8 @@ class URLRequestHttpJobTest : public ::testing::Test {
   }
 
   MockNetworkLayer network_layer_;
+  TestJobInterceptor* test_job_interceptor_;

[Randy Smith (Not in Mondays), 2016/01/11 02:27:48]

Willing to put a comment about ownership here in addition to (or instead of) the
one above?  Maybe I'm cossetting future code readers, but I at least have a knee
jerk "better look closely at that" when I see this pattern :-}.

[mmenke, 2016/01/11 06:17:11]

Done.  Always happy to add more comments.

+  URLRequestJobFactoryImpl test_job_factory_;
   TestURLRequestContext context_;
   TestDelegate delegate_;
   scoped_ptr<URLRequest> req_;
@@ -543,7 +555,7 @@ TEST_F(URLRequestHttpJobTest, TestCancelWhileReadingCookies) {
 }
 
 // Make sure that SetPriority actually sets the URLRequestHttpJob's
-// priority, both before and after start.
+// priority, before start.  Other tests handle the after start case.
 TEST_F(URLRequestHttpJobTest, SetPriorityBasic) {
   scoped_ptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(req_.get()));
   EXPECT_EQ(DEFAULT_PRIORITY, job->priority());
@@ -553,23 +565,18 @@ TEST_F(URLRequestHttpJobTest, SetPriorityBasic) {
 
   job->SetPriority(LOW);
   EXPECT_EQ(LOW, job->priority());
-
-  job->Start();
-  EXPECT_EQ(LOW, job->priority());
-
-  job->SetPriority(MEDIUM);
-  EXPECT_EQ(MEDIUM, job->priority());
 }
 
 // Make sure that URLRequestHttpJob passes on its priority to its
 // transaction on start.
 TEST_F(URLRequestHttpJobTest, SetTransactionPriorityOnStart) {
-  scoped_ptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(req_.get()));
-  job->SetPriority(LOW);
+  test_job_interceptor_->set_main_intercept_job(
+      make_scoped_ptr(new TestURLRequestHttpJob(req_.get())));
+  req_->SetPriority(LOW);
 
   EXPECT_FALSE(network_layer_.last_transaction());
 
-  job->Start();
+  req_->Start();
 
   ASSERT_TRUE(network_layer_.last_transaction());
   EXPECT_EQ(LOW, network_layer_.last_transaction()->priority());
@@ -578,13 +585,14 @@ TEST_F(URLRequestHttpJobTest, SetTransactionPriorityOnStart) {
 // Make sure that URLRequestHttpJob passes on its priority updates to
 // its transaction.
 TEST_F(URLRequestHttpJobTest, SetTransactionPriority) {
-  scoped_ptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(req_.get()));
-  job->SetPriority(LOW);
-  job->Start();
+  test_job_interceptor_->set_main_intercept_job(
+      make_scoped_ptr(new TestURLRequestHttpJob(req_.get())));
+  req_->SetPriority(LOW);
+  req_->Start();
   ASSERT_TRUE(network_layer_.last_transaction());
   EXPECT_EQ(LOW, network_layer_.last_transaction()->priority());
 
-  job->SetPriority(HIGHEST);
+  req_->SetPriority(HIGHEST);
   EXPECT_EQ(HIGHEST, network_layer_.last_transaction()->priority());
 }
 
@@ -592,8 +600,9 @@ TEST_F(URLRequestHttpJobTest, SetTransactionPriority) {
 TEST_F(URLRequestHttpJobTest, SdchAdvertisementGet) {
   EnableSdch();
   req_->set_method("GET");  // Redundant with default.
-  scoped_ptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(req_.get()));
-  job->Start();
+  test_job_interceptor_->set_main_intercept_job(
+      make_scoped_ptr(new TestURLRequestHttpJob(req_.get())));
+  req_->Start();
   EXPECT_TRUE(TransactionAcceptsSdchEncoding());
 }
 
@@ -601,8 +610,9 @@ TEST_F(URLRequestHttpJobTest, SdchAdvertisementGet) {
 TEST_F(URLRequestHttpJobTest, SdchAdvertisementPost) {
   EnableSdch();
   req_->set_method("POST");
-  scoped_ptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(req_.get()));
-  job->Start();
+  test_job_interceptor_->set_main_intercept_job(
+      make_scoped_ptr(new TestURLRequestHttpJob(req_.get())));
+  req_->Start();
   EXPECT_FALSE(TransactionAcceptsSdchEncoding());
 }
 
@@ -710,12 +720,6 @@ class URLRequestHttpJobWebSocketTest
       : req_(context_.CreateRequest(GURL("ws://www.example.com"),
                                     DEFAULT_PRIORITY,
                                     &delegate_)) {
-    // The TestNetworkDelegate expects a call to NotifyBeforeURLRequest before
-    // anything else happens.
-    GURL url("ws://localhost/");
-    TestCompletionCallback dummy;
-    network_delegate_.NotifyBeforeURLRequest(
-        req_.get(), dummy.callback(), &url);
   }
 
   TestDelegate delegate_;
@@ -741,6 +745,12 @@ class MockCreateHelper : public WebSocketHandshakeStreamBase::CreateHelper {
                                              bool));
 };
 
+// iOS doesn't support WebSockets, so these tests fail with ERR_UNKOWN_SCHEME on
+// iOS.
+// TODO(mmenke):  Hard coding features based on OS is regression prone and ugly.
+// Seems like this should use a build flag instead.
+#if !defined(OS_IOS)

[Randy Smith (Not in Mondays), 2016/01/11 02:27:48]

Why did they start failing on this CL?

[mmenke, 2016/01/11 06:17:11]

Sorry, thought I commented on it, but turns out I didn't.  The logic that we
don't support WebSockets on iOS is at the URLRequestJobFactory layer
(URLRequestJobManager, actually), so since these tests just create the job
manually, they bypass that logic.

+
 class FakeWebSocketHandshakeStream : public WebSocketHandshakeStreamBase {
  public:
   FakeWebSocketHandshakeStream() : initialize_stream_was_called_(false) {}
@@ -818,15 +828,13 @@ class FakeWebSocketHandshakeStream : public WebSocketHandshakeStreamBase {
 };
 
 TEST_F(URLRequestHttpJobWebSocketTest, RejectedWithoutCreateHelper) {
-  scoped_ptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(req_.get()));
-  job->Start();
+  req_->Start();
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(URLRequestStatus::FAILED, req_->status().status());
   EXPECT_EQ(ERR_DISALLOWED_URL_SCHEME, req_->status().error());
 }
 
 TEST_F(URLRequestHttpJobWebSocketTest, CreateHelperPassedThrough) {
-  scoped_ptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(req_.get()));
   scoped_ptr<MockCreateHelper> create_helper(
       new ::testing::StrictMock<MockCreateHelper>());
   FakeWebSocketHandshakeStream* fake_handshake_stream(
@@ -838,12 +846,14 @@ TEST_F(URLRequestHttpJobWebSocketTest, CreateHelperPassedThrough) {
   req_->SetUserData(WebSocketHandshakeStreamBase::CreateHelper::DataKey(),
                     create_helper.release());
   req_->SetLoadFlags(LOAD_DISABLE_CACHE);
-  job->Start();
+  req_->Start();
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(URLRequestStatus::IO_PENDING, req_->status().status());
   EXPECT_TRUE(fake_handshake_stream->initialize_stream_was_called());
 }
 
+#endif  // !defined(OS_IOS)
+
 }  // namespace
 
 }  // namespace net