Refactor ResourceHandler API.

content/browser/loader/mock_resource_loader.cc

Index: content/browser/loader/mock_resource_loader.cc
diff --git a/content/browser/loader/mock_resource_loader.cc b/content/browser/loader/mock_resource_loader.cc
index c47403db6be140cdc6931ce9efd1f855006f6536..de360269a9c941b64ae1acedcf5f3cb27621dae1 100644
--- a/content/browser/loader/mock_resource_loader.cc
+++ b/content/browser/loader/mock_resource_loader.cc
@@ -9,88 +9,87 @@
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "content/browser/loader/resource_controller.h"
-#include "content/browser/loader/resource_handler.h"
 #include "net/base/io_buffer.h"
 #include "net/url_request/url_request_status.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 
+class MockResourceLoader::TestResourceController : public ResourceController {
+ public:
+  explicit TestResourceController(base::WeakPtr<MockResourceLoader> mock_loader)
+      : mock_loader_(mock_loader) {}
+  ~TestResourceController() override {}
+
+  void Resume() override { mock_loader_->OnResume(); }
+
+  void Cancel() override { CancelWithError(net::ERR_ABORTED); }
+
+  void CancelAndIgnore() override {
+    ADD_FAILURE() << "Unexpected CancelAndIgnore call.";
+    Cancel();
+  }
+
+  void CancelWithError(int error_code) override {
+    mock_loader_->OnCancel(error_code);
+  }
+
+  base::WeakPtr<MockResourceLoader> mock_loader_;
+};
+
 MockResourceLoader::MockResourceLoader(ResourceHandler* resource_handler)
-    : resource_handler_(resource_handler) {
-  resource_handler_->SetController(this);
+    : resource_handler_(resource_handler), weak_factory_(this) {
+  resource_handler_->SetDelegate(this);
 }
 
 MockResourceLoader::~MockResourceLoader() {}
 
 MockResourceLoader::Status MockResourceLoader::OnWillStart(const GURL& url) {
+  EXPECT_FALSE(weak_factory_.HasWeakPtrs());
   EXPECT_EQ(Status::IDLE, status_);
-  status_ = Status::CALLING_HANDLER;
 
-  bool defer = false;
-  bool result = resource_handler_->OnWillStart(url, &defer);
-  // The second case isn't really allowed, but a number of classes do it
-  // anyways.
-  EXPECT_TRUE(status_ == Status::CALLING_HANDLER ||
-              (result == false && status_ == Status::CANCELED));
-  if (!result) {
-    status_ = Status::CANCELED;
-  } else if (defer) {
+  status_ = Status::CALLING_HANDLER;
+  resource_handler_->OnWillStart(url, base::MakeUnique<TestResourceController>(
+                                          weak_factory_.GetWeakPtr()));
+  if (status_ == Status::CALLING_HANDLER)
     status_ = Status::CALLBACK_PENDING;
-  } else {
-    status_ = Status::IDLE;
-  }
   return status_;
 }
 
 MockResourceLoader::Status MockResourceLoader::OnRequestRedirected(
     const net::RedirectInfo& redirect_info,
     ResourceResponse* response) {
+  EXPECT_FALSE(weak_factory_.HasWeakPtrs());
   EXPECT_EQ(Status::IDLE, status_);
-  status_ = Status::CALLING_HANDLER;
 
-  bool defer = false;
-  bool result =
-      resource_handler_->OnRequestRedirected(redirect_info, response, &defer);
-  // The second case isn't really allowed, but a number of classes do it
-  // anyways.
-  EXPECT_TRUE(status_ == Status::CALLING_HANDLER ||
-              (result == false && status_ == Status::CANCELED));
-  if (!result) {
-    status_ = Status::CANCELED;
-  } else if (defer) {
+  status_ = Status::CALLING_HANDLER;
+  resource_handler_->OnRequestRedirected(
+      redirect_info, response,
+      base::MakeUnique<TestResourceController>(weak_factory_.GetWeakPtr()));
+  if (status_ == Status::CALLING_HANDLER)
     status_ = Status::CALLBACK_PENDING;
-  } else {
-    status_ = Status::IDLE;
-  }
   return status_;
 }
 
 MockResourceLoader::Status MockResourceLoader::OnResponseStarted(
     ResourceResponse* response) {
+  EXPECT_FALSE(weak_factory_.HasWeakPtrs());
   EXPECT_EQ(Status::IDLE, status_);
-  status_ = Status::CALLING_HANDLER;
 
-  bool defer = false;
-  bool result = resource_handler_->OnResponseStarted(response, &defer);
-  // The second case isn't really allowed, but a number of classes do it
-  // anyways.
-  EXPECT_TRUE(status_ == Status::CALLING_HANDLER ||
-              (result == false && status_ == Status::CANCELED));
-  if (!result) {
-    status_ = Status::CANCELED;
-  } else if (defer) {
+  status_ = Status::CALLING_HANDLER;
+  resource_handler_->OnResponseStarted(
+      response,
+      base::MakeUnique<TestResourceController>(weak_factory_.GetWeakPtr()));
+  if (status_ == Status::CALLING_HANDLER)
     status_ = Status::CALLBACK_PENDING;
-  } else {
-    status_ = Status::IDLE;
-  }
   return status_;
 }
 
 MockResourceLoader::Status MockResourceLoader::OnWillRead(int min_size) {
+  EXPECT_FALSE(weak_factory_.HasWeakPtrs());
   EXPECT_EQ(Status::IDLE, status_);
-  status_ = Status::CALLING_HANDLER;
 
+  status_ = Status::CALLING_HANDLER;
   scoped_refptr<net::IOBuffer> buf;
   int buf_size;
   bool result = resource_handler_->OnWillRead(&buf, &buf_size, min_size);
@@ -108,27 +107,21 @@ MockResourceLoader::Status MockResourceLoader::OnWillRead(int min_size) {
 };
 
 MockResourceLoader::Status MockResourceLoader::OnReadCompleted(int bytes_read) {
+  EXPECT_FALSE(weak_factory_.HasWeakPtrs());
   EXPECT_EQ(Status::IDLE, status_);
-  status_ = Status::CALLING_HANDLER;
 
-  bool defer = false;
-  bool result = resource_handler_->OnReadCompleted(bytes_read, &defer);
-  // The second case isn't really allowed, but a number of classes do it
-  // anyways.
-  EXPECT_TRUE(status_ == Status::CALLING_HANDLER ||
-              (result == false && status_ == Status::CANCELED));
-  if (!result) {
-    status_ = Status::CANCELED;
-  } else if (defer) {
+  status_ = Status::CALLING_HANDLER;
+  resource_handler_->OnReadCompleted(
+      bytes_read,
+      base::MakeUnique<TestResourceController>(weak_factory_.GetWeakPtr()));
+  if (status_ == Status::CALLING_HANDLER)
     status_ = Status::CALLBACK_PENDING;
-  } else {
-    status_ = Status::IDLE;
-  }
   return status_;
 }
 
 MockResourceLoader::Status MockResourceLoader::OnResponseCompleted(
     const net::URLRequestStatus& status) {
+  EXPECT_FALSE(weak_factory_.HasWeakPtrs());
   // This should only happen while the ResourceLoader is idle or the request was
   // canceled.
   EXPECT_TRUE(status_ == Status::IDLE ||
@@ -136,45 +129,49 @@ MockResourceLoader::Status MockResourceLoader::OnResponseCompleted(
                error_code_ == status.error()));
 
   status_ = Status::CALLING_HANDLER;
-
-  bool defer = false;
-  resource_handler_->OnResponseCompleted(status, &defer);
-  EXPECT_EQ(Status::CALLING_HANDLER, status_);
-  if (defer) {
+  resource_handler_->OnResponseCompleted(
+      status,
+      base::MakeUnique<TestResourceController>(weak_factory_.GetWeakPtr()));
+  if (status_ == Status::CALLING_HANDLER)
     status_ = Status::CALLBACK_PENDING;
-  } else {
-    status_ = Status::IDLE;
-  }
+  EXPECT_NE(Status::CANCELED, status_);
   return status_;
 }
 
-void MockResourceLoader::Cancel() {
-  CancelWithError(net::ERR_ABORTED);
-}
+void MockResourceLoader::OutOfBandCancel(int error_code, bool tell_renderer) {
+  // Shouldn't be called in-band.
+  EXPECT_NE(Status::CALLING_HANDLER, status_);
+
+  status_ = Status::CANCELED;
+  canceled_out_of_band_ = true;
 
-void MockResourceLoader::CancelAndIgnore() {
-  NOTREACHED();
+  // To mimic real behavior, keep old error, in the case of double-cancel.
+  if (error_code_ == net::OK)
+    error_code_ = error_code;
 }
 
-void MockResourceLoader::CancelWithError(int error_code) {
-  EXPECT_LT(error_code, 0);
-  // Ignore double cancels.  Classes shouldn't be doing this, as
-  // ResourceLoader doesn't really support it, but they do anywways.  :(
-  // TODO(mmenke): Remove this check.
-  if (error_code_ != net::OK)
+void MockResourceLoader::OnCancel(int error_code) {
+  // It's currently allowed to be canceled in-band after being cancelled
+  // out-of-band, so do nothing, unless the status is no longer CANCELED, which
+  // which case, OnResponseCompleted has already been called, and cancels aren't
+  // expected then.
+  // TODO(mmenke):  Make CancelOutOfBand synchronously destroy the
+  // ResourceLoader.
+  if (canceled_out_of_band_ && status_ == Status::CANCELED)
     return;
 
-  // ResourceLoader really expects canceled not to be called synchronously, but
-  // a lot of code does it, so allow it.
-  // TODO(mmenke):  Remove CALLING_HANDLER.
+  EXPECT_LT(error_code, 0);
   EXPECT_TRUE(status_ == Status::CALLBACK_PENDING ||
-              status_ == Status::CALLING_HANDLER || status_ == Status::IDLE);
+              status_ == Status::CALLING_HANDLER);
+
   status_ = Status::CANCELED;
   error_code_ = error_code;
 }
 
-void MockResourceLoader::Resume() {
-  EXPECT_EQ(Status::CALLBACK_PENDING, status_);
+void MockResourceLoader::OnResume() {
+  EXPECT_TRUE(status_ == Status::CALLBACK_PENDING ||
+              status_ == Status::CALLING_HANDLER);
+
   status_ = Status::IDLE;
 }