Local NTP: add smoke tests for doodles

Split LogoService into LogoService interface and LogoServiceImpl to make
it easier to provide fake data to the test.

Bug: 768419
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I84639189d2db1b24a2e139936c99369352bab587
Reviewed-on: https://chromium-review.googlesource.com/690198Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Chris Pickel <sfiera@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505374}

chrome/browser/resources/local_ntp/local_ntp.js

@@ -622,7 +622,7 @@ function init() {
       }
 
       // Got a (possibly empty) ddl object. Show logo or doodle.
-      showLogoOrDoodle(ddl.image, ddl.metadata);
+      showLogoOrDoodle(ddl.image || null, ddl.metadata || null);
       // If we got a valid ddl object (from cache), load a fresh one.
       if (ddl.v !== null) {
         loadDoodle(ddl.v, function(ddl) {

chrome/browser/search_provider_logos/logo_service_factory.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/search_provider_logos/logo_service.h"
+#include "components/search_provider_logos/logo_service_impl.h"
 #include "net/url_request/url_request_context_getter.h"
 
 #if defined(OS_ANDROID)
@@ -18,6 +19,7 @@
 #endif
 
 using search_provider_logos::LogoService;
+using search_provider_logos::LogoServiceImpl;
 
 #if defined(OS_ANDROID)
 using chrome::android::GetIsChromeHomeEnabled;
@@ -59,8 +61,8 @@ KeyedService* LogoServiceFactory::BuildServiceInstanceFor(
 #else
   bool use_gray_background = false;
 #endif
-  return new LogoService(profile->GetPath().Append(kCachedLogoDirectory),
-                         TemplateURLServiceFactory::GetForProfile(profile),
-                         base::MakeUnique<suggestions::ImageDecoderImpl>(),
-                         profile->GetRequestContext(), use_gray_background);
+  return new LogoServiceImpl(profile->GetPath().Append(kCachedLogoDirectory),
+                             TemplateURLServiceFactory::GetForProfile(profile),
+                             base::MakeUnique<suggestions::ImageDecoderImpl>(),
+                             profile->GetRequestContext(), use_gray_background);
 }

chrome/browser/ui/search/local_ntp_browsertest.cc

@@ -9,6 +9,7 @@
 
 #include "base/command_line.h"
 #include "base/i18n/base_i18n_switches.h"
+#include "base/json/string_escape.h"
 #include "base/memory/ptr_util.h"
 #include "base/optional.h"
 #include "base/strings/utf_string_conversions.h"
@@ -27,6 +28,7 @@
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
+#include "chrome/browser/search_provider_logos/logo_service_factory.h"
 #include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -44,6 +46,7 @@
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_data.h"
 #include "components/search_engines/template_url_service.h"
+#include "components/search_provider_logos/logo_service.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
@@ -51,8 +54,17 @@
 #include "content/public/test/test_utils.h"
 #include "media/base/media_switches.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "ui/base/resource/resource_bundle.h"
 
+using search_provider_logos::EncodedLogo;
+using search_provider_logos::LogoCallbacks;
+using search_provider_logos::LogoCallbackReason;
+using search_provider_logos::LogoObserver;
+using search_provider_logos::LogoService;
+using testing::Eq;
+using testing::IsEmpty;
+
 namespace {
 
 content::WebContents* OpenNewTab(Browser* browser, const GURL& url) {
@@ -695,3 +707,148 @@ IN_PROC_BROWSER_TEST_F(LocalNTPVoiceSearchSmokeTest, MicrophonePermission) {
           GURL(chrome::kChromeUINewTabURL).GetOrigin());
   EXPECT_EQ(CONTENT_SETTING_ALLOW, mic_permission_after.content_setting);
 }
+
+// Returns configured logos.
+class FakeLogoService : public LogoService {
+ public:
+  void GetLogo(search_provider_logos::LogoCallbacks callbacks) override {
+    DCHECK(!callbacks.on_cached_decoded_logo_available);
+    DCHECK(!callbacks.on_fresh_decoded_logo_available);
+
+    if (callbacks.on_cached_encoded_logo_available) {
+      std::move(callbacks.on_cached_encoded_logo_available)
+          .Run(cached_reason, cached_logo);
+    }
+    if (callbacks.on_fresh_encoded_logo_available) {
+      std::move(callbacks.on_fresh_encoded_logo_available)
+          .Run(fresh_reason, fresh_logo);
+    }
+  }
+
+  void GetLogo(LogoObserver* observer) override { NOTREACHED(); }
+
+  LogoCallbackReason cached_reason = LogoCallbackReason::CANCELED;
+  base::Optional<EncodedLogo> cached_logo;
+  LogoCallbackReason fresh_reason = LogoCallbackReason::CANCELED;
+  base::Optional<EncodedLogo> fresh_logo;
+};
+
+class LocalNTPDoodleTest : public InProcessBrowserTest {
+ protected:
+  LocalNTPDoodleTest() {}
+
+  FakeLogoService* logo_service() {
+    return static_cast<FakeLogoService*>(
+        LogoServiceFactory::GetForProfile(browser()->profile()));
+  }
+
+  base::Optional<std::string> GetComputedStyle(content::WebContents* tab,
+                                               std::string id,
+                                               std::string css_name) {
+    std::string css_value;
+    if (instant_test_utils::GetStringFromJS(
+            tab,
+            base::StringPrintf(
+                "getComputedStyle(document.getElementById(%s))[%s]",
+                base::GetQuotedJSONString(id).c_str(),
+                base::GetQuotedJSONString(css_name).c_str()),
+            &css_value)) {
+      return css_value;
+    }
+    return base::nullopt;
+  }
+
+ private:
+  void SetUp() override {
+    feature_list_.InitWithFeatures(
+        {features::kUseGoogleLocalNtp, features::kDoodlesOnLocalNtp}, {});
+    InProcessBrowserTest::SetUp();
+  }
+
+  void SetUpInProcessBrowserTestFixture() override {
+    will_create_browser_context_services_subscription_ =
+        BrowserContextDependencyManager::GetInstance()
+            ->RegisterWillCreateBrowserContextServicesCallbackForTesting(
+                base::Bind(
+                    &LocalNTPDoodleTest::OnWillCreateBrowserContextServices,
+                    base::Unretained(this)));
+  }
+
+  static std::unique_ptr<KeyedService> CreateLogoService(
+      content::BrowserContext* context) {
+    return base::MakeUnique<FakeLogoService>();
+  }
+
+  void OnWillCreateBrowserContextServices(content::BrowserContext* context) {
+    LogoServiceFactory::GetInstance()->SetTestingFactory(
+        context, &LocalNTPDoodleTest::CreateLogoService);
+  }
+
+  base::test::ScopedFeatureList feature_list_;
+
+  std::unique_ptr<
+      base::CallbackList<void(content::BrowserContext*)>::Subscription>
+      will_create_browser_context_services_subscription_;
+};
+
+IN_PROC_BROWSER_TEST_F(LocalNTPDoodleTest,
+                       ShouldBeUnchangedOnLogoFetchCancelled) {
+  logo_service()->cached_reason = LogoCallbackReason::CANCELED;
+  logo_service()->fresh_reason = LogoCallbackReason::CANCELED;
+
+  // Open a new blank tab, then go to NTP and listen for console messages.
+  content::WebContents* active_tab = OpenNewTab(browser(), GURL("about:blank"));
+  content::ConsoleObserverDelegate console_observer(active_tab, "*");
+  active_tab->SetDelegate(&console_observer);
+  ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL));
+
+  EXPECT_THAT(GetComputedStyle(active_tab, "logo-default", "opacity"),
+              Eq<std::string>("1"));
+  EXPECT_THAT(GetComputedStyle(active_tab, "logo-doodle", "opacity"),
+              Eq<std::string>("0"));
+  EXPECT_THAT(console_observer.message(), IsEmpty());
+}
+
+IN_PROC_BROWSER_TEST_F(LocalNTPDoodleTest,
+                       ShouldBeUnchangedWhenDoodleUnavailable) {
+  logo_service()->cached_reason = LogoCallbackReason::DETERMINED;
+  logo_service()->fresh_reason = LogoCallbackReason::REVALIDATED;
+
+  // Open a new blank tab, then go to NTP and listen for console messages.
+  content::WebContents* active_tab = OpenNewTab(browser(), GURL("about:blank"));
+  content::ConsoleObserverDelegate console_observer(active_tab, "*");
+  active_tab->SetDelegate(&console_observer);
+  ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL));
+
+  EXPECT_THAT(GetComputedStyle(active_tab, "logo-default", "opacity"),
+              Eq<std::string>("1"));
+  EXPECT_THAT(GetComputedStyle(active_tab, "logo-doodle", "opacity"),
+              Eq<std::string>("0"));
+  EXPECT_THAT(console_observer.message(), IsEmpty());
+}
+
+IN_PROC_BROWSER_TEST_F(LocalNTPDoodleTest, ShouldShowDoodleWhenAvailable) {
+  logo_service()->cached_reason = LogoCallbackReason::DETERMINED;
+
+  logo_service()->cached_logo = EncodedLogo();
+  std::string encoded_image = "data:image/svg+xml,<svg/>";
+  logo_service()->cached_logo->encoded_image =
+      base::RefCountedString::TakeString(&encoded_image);
+  logo_service()->cached_logo->metadata.on_click_url =
+      GURL("https://www.chromium.org");
+  logo_service()->cached_logo->metadata.alt_text = "Chromium";
+
+  logo_service()->fresh_reason = LogoCallbackReason::REVALIDATED;
+
+  // Open a new blank tab, then go to NTP and listen for console messages.
+  content::WebContents* active_tab = OpenNewTab(browser(), GURL("about:blank"));
+  content::ConsoleObserverDelegate console_observer(active_tab, "*");
+  active_tab->SetDelegate(&console_observer);
+  ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL));
+
+  EXPECT_THAT(GetComputedStyle(active_tab, "logo-default", "opacity"),
+              Eq<std::string>("0"));
+  EXPECT_THAT(GetComputedStyle(active_tab, "logo-doodle", "opacity"),
+              Eq<std::string>("1"));
+  EXPECT_THAT(console_observer.message(), IsEmpty());
+}

components/search_provider_logos/BUILD.gn

@@ -16,6 +16,8 @@ static_library("search_provider_logos") {
     "logo_common.h",
     "logo_service.cc",
     "logo_service.h",
+    "logo_service_impl.cc",
+    "logo_service_impl.h",
     "logo_tracker.cc",
     "logo_tracker.h",
     "switches.cc",
@@ -84,7 +86,7 @@ source_set("unit_tests") {
   sources = [
     "google_logo_api_unittest.cc",
     "logo_cache_unittest.cc",
-    "logo_service_unittest.cc",
+    "logo_service_impl_unittest.cc",
   ]
   deps = [
     ":search_provider_logos",

components/search_provider_logos/logo_service.h

@@ -5,51 +5,24 @@
 #ifndef COMPONENTS_SEARCH_PROVIDER_LOGOS_ANDROID_LOGO_SERVICE_H_
 #define COMPONENTS_SEARCH_PROVIDER_LOGOS_ANDROID_LOGO_SERVICE_H_
 
-#include <memory>
-
-#include "base/files/file_path.h"
-#include "base/macros.h"
-#include "base/memory/ref_counted.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/search_provider_logos/logo_common.h"
 
-class TemplateURLService;
-
-namespace base {
-class Clock;
-}  // namespace base
-
-namespace image_fetcher {
-class ImageDecoder;
-}  // namespace image_fetcher
-
-namespace net {
-class URLRequestContextGetter;
-}  // namespace net
-
 namespace search_provider_logos {
 
-class LogoCache;
-class LogoTracker;
 class LogoObserver;
 
 // Provides the logo for a profile's default search provider.
 //
 // Example usage:
 //   LogoService* logo_service = LogoServiceFactory::GetForProfile(profile);
-//   logo_service->GetLogo(...);
+//   LogoCallbacks callbacks;
+//   callbacks.on_cached_decoded_logo = base::Bind(ShowLogo);
+//   callbacks.on_fresh_decoded_logo = base::Bind(FadeToLogo);
+//   logo_service->GetLogo(callbacks);
 //
 class LogoService : public KeyedService {
  public:
-  LogoService(
-      const base::FilePath& cache_directory,
-      TemplateURLService* template_url_service,
-      std::unique_ptr<image_fetcher::ImageDecoder> image_decoder,
-      scoped_refptr<net::URLRequestContextGetter> request_context_getter,
-      bool use_gray_background);
-
-  ~LogoService() override;
-
   // Gets the logo for the default search provider and calls the provided
   // callbacks with the encoded and decoded logos. The service will:
   //
@@ -57,7 +30,7 @@ class LogoService : public KeyedService {
   // 2.  Fetch a fresh logo, and call callbacks.on_fresh_{en,de}coded_logo.
   //
   // At least one member of |callbacks| must be non-null.
-  void GetLogo(LogoCallbacks callbacks);
+  virtual void GetLogo(LogoCallbacks callbacks) = 0;
 
   // Gets the logo for the default search provider and notifies |observer|
   // 0-2 times with the results. The service will:
@@ -69,31 +42,12 @@ class LogoService : public KeyedService {
   //     |on_fresh_decoded_logo_available| would be called in the callback
   //     interface with type DETERMINED.
   // 3.  Call observer->OnObserverRemoved().
-  void GetLogo(LogoObserver* observer);
-
-  // Overrides the cache used to store logos.
-  void SetLogoCacheForTests(std::unique_ptr<LogoCache> cache);
+  virtual void GetLogo(LogoObserver* observer) = 0;
 
-  // Overrides the clock used to check the time.
-  void SetClockForTests(std::unique_ptr<base::Clock> clock);
+ protected:
+  LogoService();
 
  private:
-  // Constructor arguments.
-  const base::FilePath cache_directory_;
-  TemplateURLService* const template_url_service_;
-  const scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
-  const bool use_gray_background_;
-
-  // logo_tracker_ takes ownership if/when it is initialized.
-  std::unique_ptr<image_fetcher::ImageDecoder> image_decoder_;
-
-  // For testing. logo_tracker_ takes ownership if/when it is initialized.
-  std::unique_ptr<base::Clock> clock_for_test_;
-  std::unique_ptr<LogoCache> logo_cache_for_test_;
-
-  // Lazily initialized on first call to GetLogo().
-  std::unique_ptr<search_provider_logos::LogoTracker> logo_tracker_;
-
   DISALLOW_COPY_AND_ASSIGN(LogoService);
 };
 

components/search_provider_logos/logo_service_impl.h

+// Copyright 2017 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.
+
+#ifndef COMPONENTS_SEARCH_PROVIDER_LOGOS_ANDROID_LOGO_SERVICE_IMPL_H_
+#define COMPONENTS_SEARCH_PROVIDER_LOGOS_ANDROID_LOGO_SERVICE_IMPL_H_
+
+#include "base/files/file_path.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "components/search_provider_logos/logo_common.h"
+#include "components/search_provider_logos/logo_service.h"
+
+class TemplateURLService;
+
+namespace base {
+class Clock;
+}  // namespace base
+
+namespace image_fetcher {
+class ImageDecoder;
+}  // namespace image_fetcher
+
+namespace net {
+class URLRequestContextGetter;
+}  // namespace net
+
+namespace search_provider_logos {
+
+class LogoCache;
+class LogoTracker;
+class LogoObserver;
+
+class LogoServiceImpl : public LogoService {
+ public:
+  LogoServiceImpl(
+      const base::FilePath& cache_directory,
+      TemplateURLService* template_url_service,
+      std::unique_ptr<image_fetcher::ImageDecoder> image_decoder,
+      scoped_refptr<net::URLRequestContextGetter> request_context_getter,
+      bool use_gray_background);
+
+  ~LogoServiceImpl() override;
+
+  // LogoService
+  void GetLogo(LogoCallbacks callbacks) override;
+  void GetLogo(LogoObserver* observer) override;
+
+  // Overrides the cache used to store logos.
+  void SetLogoCacheForTests(std::unique_ptr<LogoCache> cache);
+
+  // Overrides the clock used to check the time.
+  void SetClockForTests(std::unique_ptr<base::Clock> clock);
+
+ private:
+  // Constructor arguments.
+  const base::FilePath cache_directory_;
+  TemplateURLService* const template_url_service_;
+  const scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+  const bool use_gray_background_;
+
+  // logo_tracker_ takes ownership if/when it is initialized.
+  std::unique_ptr<image_fetcher::ImageDecoder> image_decoder_;
+
+  // For testing. logo_tracker_ takes ownership if/when it is initialized.
+  std::unique_ptr<base::Clock> clock_for_test_;
+  std::unique_ptr<LogoCache> logo_cache_for_test_;
+
+  // Lazily initialized on first call to GetLogo().
+  std::unique_ptr<search_provider_logos::LogoTracker> logo_tracker_;
+
+  DISALLOW_COPY_AND_ASSIGN(LogoServiceImpl);
+};
+
+}  // namespace search_provider_logos
+
+#endif  // COMPONENTS_SEARCH_PROVIDER_LOGOS_ANDROID_LOGO_SERVICE_IMPL_H_

ios/chrome/browser/google/google_logo_service.h

@@ -8,7 +8,7 @@
 #include <memory>
 
 #include "components/search_provider_logos/logo_common.h"
-#include "components/search_provider_logos/logo_service.h"
+#include "components/search_provider_logos/logo_service_impl.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
 // Provides the logo if a BrowserState's default search provider is Google.
@@ -20,14 +20,14 @@
 //       GoogleLogoServiceFactory::GetForBrowserState(browser_state);
 //   logo_service->GetLogo(...);
 //
-class GoogleLogoService : public search_provider_logos::LogoService {
+class GoogleLogoService : public search_provider_logos::LogoServiceImpl {
  public:
   explicit GoogleLogoService(
       TemplateURLService* template_url_service,
       scoped_refptr<net::URLRequestContextGetter> request_context_getter);
   ~GoogleLogoService() override;
 
-  using LogoService::GetLogo;
+  using LogoServiceImpl::GetLogo;
 
   // |LogoService::GetLogo| does everything on callbacks, and iOS needs to load
   // the logo immediately on page load. This caches the SkBitmap so we can

ios/chrome/browser/google/google_logo_service.mm

@@ -35,11 +35,11 @@ base::FilePath DoodleDirectory() {
 GoogleLogoService::GoogleLogoService(
     TemplateURLService* template_url_service,
     scoped_refptr<net::URLRequestContextGetter> request_context_getter)
-    : LogoService(DoodleDirectory(),
-                  template_url_service,
-                  image_fetcher::CreateIOSImageDecoder(),
-                  request_context_getter,
-                  /*use_gray_background=*/false) {}
+    : LogoServiceImpl(DoodleDirectory(),
+                      template_url_service,
+                      image_fetcher::CreateIOSImageDecoder(),
+                      request_context_getter,
+                      /*use_gray_background=*/false) {}
 
 GoogleLogoService::~GoogleLogoService() {}