[Doodle] Cleanups in DoodleFetcherImplTest

components/doodle/doodle_fetcher_impl_unittest.cc

 // Copyright 2016 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 "components/doodle/doodle_fetcher_impl.h"
 
+#include <memory>
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
+#include "base/test/mock_callback.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "components/google/core/browser/google_switches.h"
 #include "components/google/core/browser/google_url_tracker.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_request_status.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using testing::_;
+using testing::DoAll;
 using testing::Eq;
+using testing::SaveArg;
 
 namespace doodle {
 
 namespace {
 
 const char kDoodleConfigPath[] = "/async/ddljson";
 
 // Required to instantiate a GoogleUrlTracker in UNIT_TEST_MODE.
 class GoogleURLTrackerClientStub : public GoogleURLTrackerClient {
  public:
   GoogleURLTrackerClientStub() {}
   ~GoogleURLTrackerClientStub() override {}
 
   bool IsBackgroundNetworkingEnabled() override { return true; }
 
   PrefService* GetPrefs() override { return nullptr; }
 
   net::URLRequestContextGetter* GetRequestContext() override { return nullptr; }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(GoogleURLTrackerClientStub);
 };
 
-std::string Resolve(const std::string& relative_url) {
-  return GURL(GoogleURLTracker::kDefaultGoogleHomepage)
-      .Resolve(relative_url)
-      .spec();
-}
-
 void ParseJson(
     const std::string& json,
     const base::Callback<void(std::unique_ptr<base::Value> json)>& success,
     const base::Callback<void(const std::string&)>& error) {
   base::JSONReader json_reader;
   std::unique_ptr<base::Value> value = json_reader.ReadToValue(json);
   if (value) {
     success.Run(std::move(value));
   } else {
     error.Run(json_reader.GetErrorMessage());
   }
 }
 
 }  // namespace
 
 class DoodleFetcherImplTest : public testing::Test {
  public:
   DoodleFetcherImplTest()
-      : url_(GURL(GoogleURLTracker::kDefaultGoogleHomepage)),
-        google_url_tracker_(base::MakeUnique<GoogleURLTrackerClientStub>(),
+      : google_url_tracker_(base::MakeUnique<GoogleURLTrackerClientStub>(),
                             GoogleURLTracker::UNIT_TEST_MODE),
         doodle_fetcher_(
             new net::TestURLRequestContextGetter(message_loop_.task_runner()),
             &google_url_tracker_,
             base::Bind(ParseJson)) {}
 
   void RespondWithData(const std::string& data) {
-    RespondToFetcherWithData(GetRunningFetcher(), data);
-  }
-
-  void RespondToFetcherWithData(net::TestURLFetcher* url_fetcher,
-                                const std::string& data) {
+    net::TestURLFetcher* url_fetcher = GetRunningFetcher();
     url_fetcher->set_status(net::URLRequestStatus());
     url_fetcher->set_response_code(net::HTTP_OK);
     url_fetcher->SetResponseString(data);
     // Call the URLFetcher delegate to continue the test.
     url_fetcher->delegate()->OnURLFetchComplete(url_fetcher);
   }
 
   void RespondWithError(int error_code) {
     net::TestURLFetcher* url_fetcher = GetRunningFetcher();
     url_fetcher->set_status(net::URLRequestStatus::FromError(error_code));
     url_fetcher->SetResponseString("");
     // Call the URLFetcher delegate to continue the test.
     url_fetcher->delegate()->OnURLFetchComplete(url_fetcher);
   }
 
   net::TestURLFetcher* GetRunningFetcher() {
     // All created TestURLFetchers have ID 0 by default.
     net::TestURLFetcher* url_fetcher = url_fetcher_factory_.GetFetcherByID(0);
     DCHECK(url_fetcher);
     return url_fetcher;
   }
 
-  // TODO(treib): Replace with a MockCallback plus testing::SaveArgs?
-  DoodleFetcherImpl::FinishedCallback CreateResponseSavingCallback(
-      DoodleState* state_out,
-      base::TimeDelta* time_to_live_out,
-      base::Optional<DoodleConfig>* config_out) {
-    return base::BindOnce(
-        [](DoodleState* state_out, base::TimeDelta* time_to_live_out,
-           base::Optional<DoodleConfig>* config_out, DoodleState state,
-           base::TimeDelta time_to_live,
-           const base::Optional<DoodleConfig>& config) {
-          if (state_out) {
-            *state_out = state;
-          }
-          if (time_to_live_out) {
-            *time_to_live_out = time_to_live;
-          }
-          if (config_out) {
-            *config_out = config;
-          }
-        },
-        state_out, time_to_live_out, config_out);
-  }
-
   DoodleFetcherImpl* doodle_fetcher() { return &doodle_fetcher_; }
 
   GURL GetGoogleBaseURL() { return google_url_tracker_.google_url(); }
 
+  GURL Resolve(const std::string& relative_url) {
+    return GetGoogleBaseURL().Resolve(relative_url);
+  }
+
  private:
   base::MessageLoop message_loop_;
-  GURL url_;
   net::TestURLFetcherFactory url_fetcher_factory_;
   GoogleURLTracker google_url_tracker_;
   DoodleFetcherImpl doodle_fetcher_;
 };
 
 TEST_F(DoodleFetcherImplTest, ReturnsFromFetchWithoutError) {
-  DoodleState state(DoodleState::NO_DOODLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::NO_DOODLE;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, nullptr, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<2>(&response)));
   RespondWithData(R"json({"ddljson": {
         "time_to_live_ms":55000,
         "large_image": {"url":"/logos/doodles/2015/some.gif"}
       }})json");
 
   EXPECT_THAT(state, Eq(DoodleState::AVAILABLE));
   EXPECT_TRUE(response.has_value());
 }
 
 TEST_F(DoodleFetcherImplTest, ReturnsFrom404FetchWithError) {
-  DoodleState state(DoodleState::NO_DOODLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::NO_DOODLE;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, nullptr, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<2>(&response)));
   RespondWithError(net::ERR_FILE_NOT_FOUND);
 
   EXPECT_THAT(state, Eq(DoodleState::DOWNLOAD_ERROR));
   EXPECT_FALSE(response.has_value());
 }
 
 TEST_F(DoodleFetcherImplTest, ReturnsErrorForInvalidJson) {
-  DoodleState state(DoodleState::NO_DOODLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::NO_DOODLE;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, nullptr, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<2>(&response)));
   RespondWithData("}");
 
   EXPECT_THAT(state, Eq(DoodleState::PARSING_ERROR));
   EXPECT_FALSE(response.has_value());
 }
 
 TEST_F(DoodleFetcherImplTest, ReturnsErrorForIncompleteJson) {
-  DoodleState state(DoodleState::NO_DOODLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::NO_DOODLE;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, nullptr, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<2>(&response)));
   RespondWithData("{}");
 
   EXPECT_THAT(state, Eq(DoodleState::PARSING_ERROR));
   EXPECT_FALSE(response.has_value());
 }
 
 TEST_F(DoodleFetcherImplTest, ResponseContainsValidBaseInformation) {
-  DoodleState state(DoodleState::NO_DOODLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::NO_DOODLE;
   base::TimeDelta time_to_live;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, &time_to_live, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<1>(&time_to_live),
+                      SaveArg<2>(&response)));
   RespondWithData(R"json()]}'{
       "ddljson": {
         "alt_text":"Mouseover Text",
         "doodle_type":"SIMPLE",
         "interactive_html":"\u003cstyle\u003e\u003c\/style\u003e",
         "search_url":"/search?q\u003dtest",
         "target_url":"/search?q\u003dtest\u0026sa\u003dX\u0026ved\u003d0ahUKEw",
         "time_to_live_ms":55000,
         "large_image": {
           "url":"/logos/doodles/2015/new-years-eve-2015-59854387958251-hp.gif"
@@ skipping 16 matching lines @@
   EXPECT_THAT(config.interactive_html,
               Eq("\u003cstyle\u003e\u003c/style\u003e"));
 
   EXPECT_FALSE(config.large_cta_image.has_value());
   EXPECT_FALSE(config.transparent_large_image.has_value());
 
   EXPECT_THAT(time_to_live, Eq(base::TimeDelta::FromMilliseconds(55000)));
 }
 
 TEST_F(DoodleFetcherImplTest, DoodleExpiresWithinThirtyDaysForTooLargeTTL) {
-  DoodleState state(DoodleState::NO_DOODLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::NO_DOODLE;
   base::TimeDelta time_to_live;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, &time_to_live, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<1>(&time_to_live),
+                      SaveArg<2>(&response)));
   RespondWithData(R"json({"ddljson": {
       "time_to_live_ms":5184000000,
       "large_image": {"url":"/logos/doodles/2015/some.gif"}
     }})json");  // 60 days
 
   EXPECT_THAT(state, Eq(DoodleState::AVAILABLE));
   EXPECT_TRUE(response.has_value());
   EXPECT_THAT(time_to_live,
               Eq(base::TimeDelta::FromMilliseconds(30ul * 24 * 60 * 60 *
                                                    1000)));  // 30 days
 }
 
 TEST_F(DoodleFetcherImplTest, DoodleExpiresImmediatelyWithNegativeTTL) {
-  DoodleState state(DoodleState::NO_DOODLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::NO_DOODLE;
   base::TimeDelta time_to_live;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, &time_to_live, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<1>(&time_to_live),
+                      SaveArg<2>(&response)));
   RespondWithData(R"json({"ddljson": {
       "time_to_live_ms":-1,
       "large_image": {"url":"/logos/doodles/2015/some.gif"}
     }})json");
 
   EXPECT_THAT(state, Eq(DoodleState::AVAILABLE));
   EXPECT_TRUE(response.has_value());
   EXPECT_THAT(time_to_live, Eq(base::TimeDelta::FromMilliseconds(0)));
 }
 
 TEST_F(DoodleFetcherImplTest, DoodleExpiresImmediatelyWithoutValidTTL) {
-  DoodleState state(DoodleState::NO_DOODLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::NO_DOODLE;
   base::TimeDelta time_to_live;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, &time_to_live, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<1>(&time_to_live),
+                      SaveArg<2>(&response)));
   RespondWithData(R"json({"ddljson": {
         "large_image": {"url":"/logos/doodles/2015/some.gif"}
       }})json");
 
   EXPECT_THAT(state, Eq(DoodleState::AVAILABLE));
   EXPECT_TRUE(response.has_value());
   EXPECT_THAT(time_to_live, Eq(base::TimeDelta::FromMilliseconds(0)));
 }
 
 TEST_F(DoodleFetcherImplTest, ReturnsNoDoodleForMissingLargeImageUrl) {
-  DoodleState state(DoodleState::AVAILABLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::AVAILABLE;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, nullptr, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<2>(&response)));
   RespondWithData(R"json({"ddljson": {
         "time_to_live_ms":55000,
         "large_image": {}
       }})json");
 
   EXPECT_THAT(state, Eq(DoodleState::NO_DOODLE));
   EXPECT_FALSE(response.has_value());
 }
 
 TEST_F(DoodleFetcherImplTest, EmptyResponsesCausesNoDoodleState) {
-  DoodleState state(DoodleState::AVAILABLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::AVAILABLE;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, nullptr, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<2>(&response)));
   RespondWithData("{\"ddljson\":{}}");
 
   EXPECT_THAT(state, Eq(DoodleState::NO_DOODLE));
   EXPECT_FALSE(response.has_value());
 }
 
 TEST_F(DoodleFetcherImplTest, ResponseContainsExactlyTheSampleImages) {
-  DoodleState state(DoodleState::NO_DOODLE);
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
+
+  DoodleState state = DoodleState::NO_DOODLE;
   base::Optional<DoodleConfig> response;
-
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state, nullptr, &response));
+  EXPECT_CALL(callback, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state), SaveArg<2>(&response)));
   RespondWithData(R"json()]}'{
       "ddljson": {
         "time_to_live_ms":55000,
         "large_image": {
           "height":225,
           "is_animated_gif":true,
           "is_cta":false,
           "url":"/logos/doodles/2015/new-years-eve-2015-59854387958251-hp.gif",
           "width":489
         },
@@ skipping 11 matching lines @@
           "url":"/logos/doodles/2015/new-years-eve-2015-59854387958251-thp.png",
           "width":510
         }
       }})json");
 
   EXPECT_THAT(state, Eq(DoodleState::AVAILABLE));
   ASSERT_TRUE(response.has_value());
   DoodleConfig config = response.value();
 
   EXPECT_TRUE(config.large_image.url.is_valid());
-  EXPECT_THAT(config.large_image.url.spec(),
+  EXPECT_THAT(config.large_image.url,
               Eq(Resolve("/logos/doodles/2015/new-years-eve-2015-5985438795"
                          "8251-hp.gif")));
   EXPECT_THAT(config.large_image.width, Eq(489));
   EXPECT_THAT(config.large_image.height, Eq(225));
   EXPECT_TRUE(config.large_image.is_animated_gif);
   EXPECT_FALSE(config.large_image.is_cta);
 
   ASSERT_TRUE(config.transparent_large_image.has_value());
   EXPECT_TRUE(config.transparent_large_image->url.is_valid());
-  EXPECT_THAT(config.transparent_large_image->url.spec(),
+  EXPECT_THAT(config.transparent_large_image->url,
               Eq(Resolve("/logos/doodles/2015/new-years-eve-2015-5985438795"
                          "8251-thp.png")));
   EXPECT_THAT(config.transparent_large_image->width, Eq(510));
   EXPECT_THAT(config.transparent_large_image->height, Eq(225));
   EXPECT_FALSE(config.transparent_large_image->is_animated_gif);
   EXPECT_FALSE(config.transparent_large_image->is_cta);
 
   ASSERT_TRUE(config.large_cta_image.has_value());
   EXPECT_TRUE(config.large_cta_image->url.is_valid());
-  EXPECT_THAT(config.large_cta_image->url.spec(),
+  EXPECT_THAT(config.large_cta_image->url,
               Eq(Resolve("/logos/doodles/2015/new-years-eve-2015-5985438795"
                          "8251-cta.gif")));
   EXPECT_THAT(config.large_cta_image->width, Eq(489));
   EXPECT_THAT(config.large_cta_image->height, Eq(225));
   EXPECT_TRUE(config.large_cta_image->is_animated_gif);
   EXPECT_TRUE(config.large_cta_image->is_cta);
 }
 
 TEST_F(DoodleFetcherImplTest, RespondsToMultipleRequestsWithSameFetcher) {
-  DoodleState state1(DoodleState::NO_DOODLE);
-  DoodleState state2(DoodleState::NO_DOODLE);
-  base::Optional<DoodleConfig> response1;
-  base::Optional<DoodleConfig> response2;
-
   // Trigger two requests.
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state1, nullptr, &response1));
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback1;
+  doodle_fetcher()->FetchDoodle(callback1.Get());
   net::URLFetcher* first_created_fetcher = GetRunningFetcher();
-  doodle_fetcher()->FetchDoodle(
-      CreateResponseSavingCallback(&state2, nullptr, &response2));
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback2;
+  doodle_fetcher()->FetchDoodle(callback2.Get());
   net::URLFetcher* second_created_fetcher = GetRunningFetcher();
 
   // Expect that only one fetcher handles both requests.
   EXPECT_THAT(first_created_fetcher, Eq(second_created_fetcher));
 
+  // But both callbacks should get called.
+  DoodleState state1 = DoodleState::NO_DOODLE;
+  DoodleState state2 = DoodleState::NO_DOODLE;
+  base::Optional<DoodleConfig> response1;
+  base::Optional<DoodleConfig> response2;
+
+  EXPECT_CALL(callback1, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state1), SaveArg<2>(&response1)));
+  EXPECT_CALL(callback2, Run(_, _, _))
+      .WillOnce(DoAll(SaveArg<0>(&state2), SaveArg<2>(&response2)));
+
   RespondWithData(R"json({"ddljson": {
         "time_to_live_ms":55000,
         "large_image": {"url":"/logos/doodles/2015/some.gif"}
       }})json");
 
   // Ensure that both requests received a response.
   EXPECT_THAT(state1, Eq(DoodleState::AVAILABLE));
   EXPECT_TRUE(response1.has_value());
   EXPECT_THAT(state2, Eq(DoodleState::AVAILABLE));
   EXPECT_TRUE(response2.has_value());
 }
 
 TEST_F(DoodleFetcherImplTest, ReceivesBaseUrlFromTracker) {
-  doodle_fetcher()->FetchDoodle(CreateResponseSavingCallback(
-      /*state=*/nullptr, nullptr, /*response=*/nullptr));
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
 
+  // TODO(treib,fhorschig): This doesn't really test anything useful, since the
+  // Google base URL is the default anyway. Find a way to set the base URL in
+  // the tracker.
   EXPECT_THAT(GetRunningFetcher()->GetOriginalURL(),
               Eq(GetGoogleBaseURL().Resolve(kDoodleConfigPath)));
 }
 
 TEST_F(DoodleFetcherImplTest, OverridesBaseUrlWithCommandLineArgument) {
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kGoogleBaseURL, "http://www.google.kz");
 
-  doodle_fetcher()->FetchDoodle(CreateResponseSavingCallback(
-      /*state=*/nullptr, nullptr, /*response=*/nullptr));
+  base::MockCallback<DoodleFetcherImpl::FinishedCallback> callback;
+  doodle_fetcher()->FetchDoodle(callback.Get());
 
   EXPECT_THAT(GetRunningFetcher()->GetOriginalURL(),
               Eq(GURL("http://www.google.kz").Resolve(kDoodleConfigPath)));
 }
 
 }  // namespace doodle