Simplify some PrivetURLFetcher::Delegate methods.

Change-Id: I5dba826a1ff92c0032e1de442c206497a318b91c
Reviewed-on: https://chromium-review.googlesource.com/805154
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521513}

chrome/browser/printing/cloud_print/privet_http_impl.cc

@@ -113,7 +113,7 @@ PrivetHTTPClient* PrivetInfoOperationImpl::GetHTTPClient() {
   return privet_client_;
 }
 
-void PrivetInfoOperationImpl::OnError(PrivetURLFetcher* fetcher,
+void PrivetInfoOperationImpl::OnError(int response_code,
                                       PrivetURLFetcher::ErrorType error) {
   callback_.Run(nullptr);
 }
@@ -173,14 +173,14 @@ PrivetHTTPClient* PrivetRegisterOperationImpl::GetHTTPClient() {
   return privet_client_;
 }
 
-void PrivetRegisterOperationImpl::OnError(PrivetURLFetcher* fetcher,
+void PrivetRegisterOperationImpl::OnError(int response_code,
                                           PrivetURLFetcher::ErrorType error) {
   ongoing_ = false;
   int visible_http_code = -1;
   FailureReason reason = FAILURE_NETWORK;
 
   if (error == PrivetURLFetcher::RESPONSE_CODE_ERROR) {
-    visible_http_code = fetcher->response_code();
+    visible_http_code = response_code;
     reason = FAILURE_HTTP_ERROR;
   } else if (error == PrivetURLFetcher::JSON_PARSE_ERROR) {
     reason = FAILURE_MALFORMED_RESPONSE;
@@ -218,7 +218,6 @@ void PrivetRegisterOperationImpl::OnParsedJson(
 }
 
 void PrivetRegisterOperationImpl::OnNeedPrivetToken(
-    PrivetURLFetcher* fetcher,
     const PrivetURLFetcher::TokenCallback& callback) {
   privet_client_->RefreshPrivetToken(callback);
 }
@@ -319,9 +318,8 @@ PrivetRegisterOperationImpl::Cancelation::~Cancelation() {
 }
 
 void PrivetRegisterOperationImpl::Cancelation::OnError(
-    PrivetURLFetcher* fetcher,
-    PrivetURLFetcher::ErrorType error) {
-}
+    int response_code,
+    PrivetURLFetcher::ErrorType error) {}
 
 void PrivetRegisterOperationImpl::Cancelation::OnParsedJson(
     PrivetURLFetcher* fetcher,
@@ -359,9 +357,8 @@ PrivetHTTPClient* PrivetJSONOperationImpl::GetHTTPClient() {
   return privet_client_;
 }
 
-void PrivetJSONOperationImpl::OnError(
-    PrivetURLFetcher* fetcher,
-    PrivetURLFetcher::ErrorType error) {
+void PrivetJSONOperationImpl::OnError(int response_code,
+                                      PrivetURLFetcher::ErrorType error) {
   callback_.Run(nullptr);
 }
 
@@ -372,7 +369,6 @@ void PrivetJSONOperationImpl::OnParsedJson(PrivetURLFetcher* fetcher,
 }
 
 void PrivetJSONOperationImpl::OnNeedPrivetToken(
-    PrivetURLFetcher* fetcher,
     const PrivetURLFetcher::TokenCallback& callback) {
   privet_client_->RefreshPrivetToken(callback);
 }
@@ -611,9 +607,8 @@ PrivetHTTPClient* PrivetLocalPrintOperationImpl::GetHTTPClient() {
   return privet_client_;
 }
 
-void PrivetLocalPrintOperationImpl::OnError(
-    PrivetURLFetcher* fetcher,
-    PrivetURLFetcher::ErrorType error) {
+void PrivetLocalPrintOperationImpl::OnError(int response_code,
+                                            PrivetURLFetcher::ErrorType error) {
   delegate_->OnPrivetPrintingError(this, -1);
 }
 
@@ -626,7 +621,6 @@ void PrivetLocalPrintOperationImpl::OnParsedJson(
 }
 
 void PrivetLocalPrintOperationImpl::OnNeedPrivetToken(
-    PrivetURLFetcher* fetcher,
     const PrivetURLFetcher::TokenCallback& callback) {
   privet_client_->RefreshPrivetToken(callback);
 }

chrome/browser/printing/cloud_print/privet_http_impl.h

@@ -34,8 +34,7 @@ class PrivetInfoOperationImpl : public PrivetJSONOperation,
 
   PrivetHTTPClient* GetHTTPClient() override;
 
-  void OnError(PrivetURLFetcher* fetcher,
-               PrivetURLFetcher::ErrorType error) override;
+  void OnError(int response_code, PrivetURLFetcher::ErrorType error) override;
   void OnParsedJson(PrivetURLFetcher* fetcher,
                     const base::DictionaryValue& value,
                     bool has_error) override;
@@ -60,15 +59,13 @@ class PrivetRegisterOperationImpl
   void Cancel() override;
   void CompleteRegistration() override;
 
-  void OnError(PrivetURLFetcher* fetcher,
-               PrivetURLFetcher::ErrorType error) override;
+  void OnError(int response_code, PrivetURLFetcher::ErrorType error) override;
 
   void OnParsedJson(PrivetURLFetcher* fetcher,
                     const base::DictionaryValue& value,
                     bool has_error) override;
 
   void OnNeedPrivetToken(
-      PrivetURLFetcher* fetcher,
       const PrivetURLFetcher::TokenCallback& callback) override;
 
   PrivetHTTPClient* GetHTTPClient() override;
@@ -79,8 +76,7 @@ class PrivetRegisterOperationImpl
     Cancelation(PrivetHTTPClient* privet_client, const std::string& user);
     ~Cancelation() override;
 
-    void OnError(PrivetURLFetcher* fetcher,
-                 PrivetURLFetcher::ErrorType error) override;
+    void OnError(int response_code, PrivetURLFetcher::ErrorType error) override;
 
     void OnParsedJson(PrivetURLFetcher* fetcher,
                       const base::DictionaryValue& value,
@@ -131,13 +127,11 @@ class PrivetJSONOperationImpl : public PrivetJSONOperation,
 
   PrivetHTTPClient* GetHTTPClient() override;
 
-  void OnError(PrivetURLFetcher* fetcher,
-               PrivetURLFetcher::ErrorType error) override;
+  void OnError(int response_code, PrivetURLFetcher::ErrorType error) override;
   void OnParsedJson(PrivetURLFetcher* fetcher,
                     const base::DictionaryValue& value,
                     bool has_error) override;
   void OnNeedPrivetToken(
-      PrivetURLFetcher* fetcher,
       const PrivetURLFetcher::TokenCallback& callback) override;
 
  private:
@@ -172,13 +166,11 @@ class PrivetLocalPrintOperationImpl
   PrivetHTTPClient* GetHTTPClient() override;
 
   // PrivetURLFetcher::Delegate:
-  void OnError(PrivetURLFetcher* fetcher,
-               PrivetURLFetcher::ErrorType error) override;
+  void OnError(int response_code, PrivetURLFetcher::ErrorType error) override;
   void OnParsedJson(PrivetURLFetcher* fetcher,
                     const base::DictionaryValue& value,
                     bool has_error) override;
   void OnNeedPrivetToken(
-      PrivetURLFetcher* fetcher,
       const PrivetURLFetcher::TokenCallback& callback) override;
 
  private:

chrome/browser/printing/cloud_print/privet_http_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 #include <utility>
+#include <vector>
 
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
@@ -1056,13 +1057,11 @@ class PrivetHttpWithServerTest : public ::testing::Test,
   }
 
   void OnNeedPrivetToken(
-      PrivetURLFetcher* fetcher,
       const PrivetURLFetcher::TokenCallback& callback) override {
     callback.Run("abc");
   }
 
-  void OnError(PrivetURLFetcher* fetcher,
-               PrivetURLFetcher::ErrorType error) override {
+  void OnError(int response_code, PrivetURLFetcher::ErrorType error) override {
     done_ = true;
     success_ = false;
     error_ = error;
@@ -1077,8 +1076,7 @@ class PrivetHttpWithServerTest : public ::testing::Test,
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, quit_);
   }
 
-  bool OnRawData(PrivetURLFetcher* fetcher,
-                 bool response_is_file,
+  bool OnRawData(bool response_is_file,
                  const std::string& data_string,
                  const base::FilePath& data_file) override {
     done_ = true;

chrome/browser/printing/cloud_print/privet_url_fetcher.cc

@@ -8,6 +8,7 @@
 
 #include <algorithm>
 #include <limits>
+#include <map>
 #include <memory>
 
 #include "base/bind.h"
@@ -58,13 +59,11 @@ std::string MakeRangeHeader(int start, int end) {
 }  // namespace
 
 void PrivetURLFetcher::Delegate::OnNeedPrivetToken(
-    PrivetURLFetcher* fetcher,
     const TokenCallback& callback) {
-  OnError(fetcher, TOKEN_ERROR);
+  OnError(0, TOKEN_ERROR);
 }
 
-bool PrivetURLFetcher::Delegate::OnRawData(PrivetURLFetcher* fetcher,
-                                           bool response_is_file,
+bool PrivetURLFetcher::Delegate::OnRawData(bool response_is_file,
                                            const std::string& data_string,
                                            const base::FilePath& data_file) {
   return false;
@@ -191,16 +190,15 @@ void PrivetURLFetcher::Try() {
 
     url_fetcher_->Start();
   } else {
-    delegate_->OnError(this, UNKNOWN_ERROR);
+    delegate_->OnError(0, UNKNOWN_ERROR);
   }
 }
 
 void PrivetURLFetcher::Start() {
   DCHECK_EQ(tries_, 0);  // We haven't called |Start()| yet.
 
-  if (!url_.is_valid()) {
-    return delegate_->OnError(this, UNKNOWN_ERROR);
-  }
+  if (!url_.is_valid())
+    return delegate_->OnError(0, UNKNOWN_ERROR);
 
   if (!send_empty_privet_token_) {
     std::string privet_access_token;
@@ -256,7 +254,7 @@ void PrivetURLFetcher::OnURLFetchComplete(const net::URLFetcher* source) {
 bool PrivetURLFetcher::OnURLFetchCompleteDoNotParseData(
     const net::URLFetcher* source) {
   if (source->GetStatus().status() == net::URLRequestStatus::CANCELED) {
-    delegate_->OnError(this, REQUEST_CANCELED);
+    delegate_->OnError(0, REQUEST_CANCELED);
     return true;
   }
 
@@ -268,7 +266,7 @@ bool PrivetURLFetcher::OnURLFetchCompleteDoNotParseData(
   if (source->GetResponseCode() != net::HTTP_OK &&
       source->GetResponseCode() != net::HTTP_PARTIAL_CONTENT &&
       source->GetResponseCode() != net::HTTP_BAD_REQUEST) {
-    delegate_->OnError(this, RESPONSE_CODE_ERROR);
+    delegate_->OnError(response_code(), RESPONSE_CODE_ERROR);
     return true;
   }
 
@@ -276,21 +274,20 @@ bool PrivetURLFetcher::OnURLFetchCompleteDoNotParseData(
     base::FilePath response_file_path;
 
     if (!source->GetResponseAsFilePath(true, &response_file_path)) {
-      delegate_->OnError(this, UNKNOWN_ERROR);
+      delegate_->OnError(0, UNKNOWN_ERROR);
       return true;
     }
 
-    return delegate_->OnRawData(this, true, std::string(), response_file_path);
-  } else {
-    std::string response_str;
-
-    if (!source->GetResponseAsString(&response_str)) {
-      delegate_->OnError(this, UNKNOWN_ERROR);
-      return true;
-    }
+    return delegate_->OnRawData(true, std::string(), response_file_path);
+  }
 
-    return delegate_->OnRawData(this, false, response_str, base::FilePath());
+  std::string response_str;
+  if (!source->GetResponseAsString(&response_str)) {
+    delegate_->OnError(0, UNKNOWN_ERROR);
+    return true;
   }
+
+  return delegate_->OnRawData(false, response_str, base::FilePath());
 }
 
 void PrivetURLFetcher::OnURLFetchCompleteParseData(
@@ -298,27 +295,27 @@ void PrivetURLFetcher::OnURLFetchCompleteParseData(
   // Response contains error description.
   bool is_error_response = false;
   if (source->GetResponseCode() != net::HTTP_OK) {
-    delegate_->OnError(this, RESPONSE_CODE_ERROR);
+    delegate_->OnError(response_code(), RESPONSE_CODE_ERROR);
     return;
   }
 
   std::string response_str;
   if (!source->GetResponseAsString(&response_str)) {
-    delegate_->OnError(this, UNKNOWN_ERROR);
+    delegate_->OnError(0, UNKNOWN_ERROR);
     return;
   }
 
   base::JSONReader json_reader(base::JSON_ALLOW_TRAILING_COMMAS);
   std::unique_ptr<base::Value> value = json_reader.ReadToValue(response_str);
   if (!value) {
-    delegate_->OnError(this, JSON_PARSE_ERROR);
+    delegate_->OnError(0, JSON_PARSE_ERROR);
     return;
   }
 
   const base::DictionaryValue* dictionary_value = NULL;
 
   if (!value->GetAsDictionary(&dictionary_value)) {
-    delegate_->OnError(this, JSON_PARSE_ERROR);
+    delegate_->OnError(0, JSON_PARSE_ERROR);
     return;
   }
 
@@ -367,13 +364,12 @@ void PrivetURLFetcher::ScheduleRetry(int timeout_seconds) {
 
 void PrivetURLFetcher::RequestTokenRefresh() {
   delegate_->OnNeedPrivetToken(
-      this,
       base::Bind(&PrivetURLFetcher::RefreshToken, weak_factory_.GetWeakPtr()));
 }
 
 void PrivetURLFetcher::RefreshToken(const std::string& token) {
   if (token.empty()) {
-    delegate_->OnError(this, TOKEN_ERROR);
+    delegate_->OnError(0, TOKEN_ERROR);
   } else {
     SetTokenForHost(GetHostString(), token);
     Try();

chrome/browser/printing/cloud_print/privet_url_fetcher.h

@@ -46,20 +46,18 @@ class PrivetURLFetcher : public net::URLFetcherDelegate {
 
     // If you do not implement this method for PrivetV1 callers, you will always
     // get a TOKEN_ERROR error when your token is invalid.
-    virtual void OnNeedPrivetToken(
-        PrivetURLFetcher* fetcher,
-        const TokenCallback& callback);
+    virtual void OnNeedPrivetToken(const TokenCallback& callback);
 
-    virtual void OnError(PrivetURLFetcher* fetcher, ErrorType error) = 0;
+    // |response_code| is only needed for RESPONSE_CODE_ERROR.
+    virtual void OnError(int response_code, ErrorType error) = 0;
     virtual void OnParsedJson(PrivetURLFetcher* fetcher,
                               const base::DictionaryValue& value,
                               bool has_error) = 0;
 
     // If this method is returns true, the data will not be parsed as JSON, and
-    // |OnParsedJson| will not be called. Otherwise, |OnParsedJson| will be
+    // OnParsedJson() will not be called. Otherwise, OnParsedJson() will be
     // called.
-    virtual bool OnRawData(PrivetURLFetcher* fetcher,
-                           bool response_is_file,
+    virtual bool OnRawData(bool response_is_file,
                            const std::string& data_string,
                            const base::FilePath& data_file);
   };
@@ -87,11 +85,11 @@ class PrivetURLFetcher : public net::URLFetcherDelegate {
 
   void SendEmptyPrivetToken();
 
-  // Set the contents of the Range header. |OnRawData| must return true if this
+  // Set the contents of the Range header. OnRawData() must return true if this
   // is called.
   void SetByteRange(int start, int end);
 
-  // Save the response to a file. |OnRawData| must return true if this is
+  // Save the response to a file. OnRawData() must return true if this is
   // called.
   void SaveResponseToFile();
 

chrome/browser/printing/cloud_print/privet_url_fetcher_unittest.cc

@@ -40,8 +40,7 @@ class MockPrivetURLFetcherDelegate : public PrivetURLFetcher::Delegate {
   ~MockPrivetURLFetcherDelegate() override {
   }
 
-  void OnError(PrivetURLFetcher* fetcher,
-               PrivetURLFetcher::ErrorType error) override {
+  void OnError(int response_code, PrivetURLFetcher::ErrorType error) override {
     OnErrorInternal(error);
   }
 
@@ -57,12 +56,10 @@ class MockPrivetURLFetcherDelegate : public PrivetURLFetcher::Delegate {
   MOCK_METHOD1(OnParsedJsonInternal, void(bool has_error));
 
   virtual void OnNeedPrivetToken(
-      PrivetURLFetcher* fetcher,
       const PrivetURLFetcher::TokenCallback& callback) {
   }
 
-  bool OnRawData(PrivetURLFetcher* fetcher,
-                 bool response_is_file,
+  bool OnRawData(bool response_is_file,
                  const std::string& data,
                  const base::FilePath& response_file) {
     if (!raw_mode_) return false;
@@ -130,7 +127,7 @@ class PrivetURLFetcherTest : public ::testing::Test {
 TEST_F(PrivetURLFetcherTest, FetchSuccess) {
   privet_urlfetcher_->Start();
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   fetcher->SetResponseString(kSampleParsableJSON);
   fetcher->set_status(net::URLRequestStatus(net::URLRequestStatus::SUCCESS,
                                             net::OK));
@@ -141,7 +138,7 @@ TEST_F(PrivetURLFetcherTest, FetchSuccess) {
 
   const base::DictionaryValue* value = delegate_.saved_value();
   int hello_value;
-  ASSERT_TRUE(value != NULL);
+  ASSERT_TRUE(value);
   ASSERT_TRUE(value->GetInteger("hello", &hello_value));
   EXPECT_EQ(2, hello_value);
 }
@@ -149,7 +146,7 @@ TEST_F(PrivetURLFetcherTest, FetchSuccess) {
 TEST_F(PrivetURLFetcherTest, HTTP503Retry) {
   privet_urlfetcher_->Start();
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   fetcher->SetResponseString(kSampleParsableJSON);
   fetcher->set_status(net::URLRequestStatus(net::URLRequestStatus::SUCCESS,
                                             net::OK));
@@ -160,7 +157,7 @@ TEST_F(PrivetURLFetcherTest, HTTP503Retry) {
   RunFor(base::TimeDelta::FromSeconds(7));
   fetcher = fetcher_factory_.GetFetcherByID(0);
 
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   fetcher->SetResponseString(kSampleParsableJSON);
   fetcher->set_status(net::URLRequestStatus(net::URLRequestStatus::SUCCESS,
                                             net::OK));
@@ -173,7 +170,7 @@ TEST_F(PrivetURLFetcherTest, HTTP503Retry) {
 TEST_F(PrivetURLFetcherTest, ResponseCodeError) {
   privet_urlfetcher_->Start();
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   fetcher->SetResponseString(kSampleParsableJSON);
   fetcher->set_status(net::URLRequestStatus(net::URLRequestStatus::SUCCESS,
                                             net::OK));
@@ -187,7 +184,7 @@ TEST_F(PrivetURLFetcherTest, ResponseCodeError) {
 TEST_F(PrivetURLFetcherTest, JsonParseError) {
   privet_urlfetcher_->Start();
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   fetcher->SetResponseString(kSampleUnparsableJSON);
   fetcher->set_status(net::URLRequestStatus(net::URLRequestStatus::SUCCESS,
                                             net::OK));
@@ -201,7 +198,7 @@ TEST_F(PrivetURLFetcherTest, JsonParseError) {
 TEST_F(PrivetURLFetcherTest, Header) {
   privet_urlfetcher_->Start();
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   net::HttpRequestHeaders headers;
   fetcher->GetExtraRequestHeaders(&headers);
 
@@ -218,7 +215,7 @@ TEST_F(PrivetURLFetcherTest, Header2) {
   privet_urlfetcher_->Start();
 
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   net::HttpRequestHeaders headers;
   fetcher->GetExtraRequestHeaders(&headers);
 
@@ -235,7 +232,7 @@ TEST_F(PrivetURLFetcherTest, AlwaysSendEmpty) {
   privet_urlfetcher_->Start();
 
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   net::HttpRequestHeaders headers;
   fetcher->GetExtraRequestHeaders(&headers);
 
@@ -247,7 +244,7 @@ TEST_F(PrivetURLFetcherTest, AlwaysSendEmpty) {
 TEST_F(PrivetURLFetcherTest, FetchHasError) {
   privet_urlfetcher_->Start();
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   fetcher->SetResponseString(kSampleJSONWithError);
   fetcher->set_status(net::URLRequestStatus(net::URLRequestStatus::SUCCESS,
                                             net::OK));
@@ -261,7 +258,7 @@ TEST_F(PrivetURLFetcherTest, FetcherRawData) {
   delegate_.SetRawMode(true);
   privet_urlfetcher_->Start();
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   fetcher->SetResponseString(kSampleJSONWithError);
   fetcher->set_status(net::URLRequestStatus(net::URLRequestStatus::SUCCESS,
                                             net::OK));
@@ -276,7 +273,7 @@ TEST_F(PrivetURLFetcherTest, RangeRequest) {
   privet_urlfetcher_->SetByteRange(200, 300);
   privet_urlfetcher_->Start();
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   net::HttpRequestHeaders headers;
   fetcher->GetExtraRequestHeaders(&headers);
 
@@ -290,7 +287,7 @@ TEST_F(PrivetURLFetcherTest, FetcherToFile) {
   privet_urlfetcher_->SaveResponseToFile();
   privet_urlfetcher_->Start();
   net::TestURLFetcher* fetcher = fetcher_factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher != NULL);
+  ASSERT_TRUE(fetcher);
   fetcher->SetResponseFilePath(
       base::FilePath(FILE_PATH_LITERAL("sample/file")));
   fetcher->set_status(net::URLRequestStatus(net::URLRequestStatus::SUCCESS,