Remove UseNewDoodleApi feature

This removes the plumbing in LogoBridge for using components/doodle.
The actual components/doodle code will be deleted separately later.

TBRing deletion of the c/b/doodle folder (which I in fact own, but the
commit queue doesn't seem to care :)
TBR=jochen

Bug: 690467
Change-Id: Ie9b52bb3ab79f14795cbdd70d3fba4b9ada5032f
Reviewed-on: https://chromium-review.googlesource.com/544311
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481875}

chrome/browser/BUILD.gn

@@ -333,8 +333,6 @@ split_static_library("browser") {
     "dom_distiller/tab_utils.h",
     "domain_reliability/service_factory.cc",
     "domain_reliability/service_factory.h",
-    "doodle/doodle_service_factory.cc",
-    "doodle/doodle_service_factory.h",
     "download/chrome_download_manager_delegate.cc",
     "download/chrome_download_manager_delegate.h",
     "download/download_confirmation_reason.h",

chrome/browser/about_flags.cc

@@ -928,56 +928,6 @@ const FeatureEntry::FeatureVariation
          arraysize(kAutofillCreditCardLastUsedDateFeatureVariationExpDate),
          nullptr}};
 
-#if defined(OS_ANDROID)
-const FeatureEntry::FeatureParam kUseNewDoodleApiTest0[] = {
-    {"doodle_override_url",
-     "https://www.gstatic.com/chrome/ntp/doodle_test/android0.json"}};
-const FeatureEntry::FeatureParam kUseNewDoodleApiTest1[] = {
-    {"doodle_override_url",
-     "https://www.gstatic.com/chrome/ntp/doodle_test/android1.json"}};
-const FeatureEntry::FeatureParam kUseNewDoodleApiTest2[] = {
-    {"doodle_override_url",
-     "https://www.gstatic.com/chrome/ntp/doodle_test/android2.json"}};
-const FeatureEntry::FeatureParam kUseNewDoodleApiTest3[] = {
-    {"doodle_override_url",
-     "https://www.gstatic.com/chrome/ntp/doodle_test/android3.json"}};
-const FeatureEntry::FeatureParam kUseNewDoodleApiTest4[] = {
-    {"doodle_override_url",
-     "https://www.gstatic.com/chrome/ntp/doodle_test/android4.json"}};
-const FeatureEntry::FeatureParam kUseNewDoodleApiTest5[] = {
-    {"doodle_override_url",
-     "https://www.gstatic.com/chrome/ntp/doodle_test/android5.json"}};
-const FeatureEntry::FeatureParam kUseNewDoodleApiTest6[] = {
-    {"doodle_override_url",
-     "https://www.gstatic.com/chrome/ntp/doodle_test/android6.json"}};
-const FeatureEntry::FeatureParam kUseNewDoodleApiTest7[] = {
-    {"doodle_override_url",
-     "https://www.gstatic.com/chrome/ntp/doodle_test/android7.json"}};
-const FeatureEntry::FeatureParam kUseNewDoodleApiTest8[] = {
-    {"doodle_override_url",
-     "https://www.gstatic.com/chrome/ntp/doodle_test/android8.json"}};
-
-const FeatureEntry::FeatureVariation kUseNewDoodleApiVariations[] = {
-    {"(force test doodle 0)", kUseNewDoodleApiTest0,
-     arraysize(kUseNewDoodleApiTest0), nullptr},
-    {"(force test doodle 1)", kUseNewDoodleApiTest1,
-     arraysize(kUseNewDoodleApiTest1), nullptr},
-    {"(force test doodle 2)", kUseNewDoodleApiTest2,
-     arraysize(kUseNewDoodleApiTest2), nullptr},
-    {"(force test doodle 3)", kUseNewDoodleApiTest3,
-     arraysize(kUseNewDoodleApiTest3), nullptr},
-    {"(force test doodle 4)", kUseNewDoodleApiTest4,
-     arraysize(kUseNewDoodleApiTest4), nullptr},
-    {"(force test doodle 5)", kUseNewDoodleApiTest5,
-     arraysize(kUseNewDoodleApiTest5), nullptr},
-    {"(force test doodle 6)", kUseNewDoodleApiTest6,
-     arraysize(kUseNewDoodleApiTest6), nullptr},
-    {"(force test doodle 7)", kUseNewDoodleApiTest7,
-     arraysize(kUseNewDoodleApiTest7), nullptr},
-    {"(force test doodle 8)", kUseNewDoodleApiTest8,
-     arraysize(kUseNewDoodleApiTest8), nullptr}};
-#endif  // OS_ANDROID
-
 const FeatureEntry::FeatureParam kMemoryAblation5MiB_512[] = {
     {kMemoryAblationFeatureSizeParam, "5242880"},
     {kMemoryAblationFeatureMaxRAMParam, "512"}};
@@ -2889,14 +2839,6 @@ const FeatureEntry kFeatureEntries[] = {
      MULTI_VALUE_TYPE(kAshForceTabletModeChoices)},
 #endif  // OS_CHROMEOS
 
-#if defined(OS_ANDROID)
-    {"use-new-doodle-api", flag_descriptions::kUseNewDoodleApiName,
-     flag_descriptions::kUseNewDoodleApiDescription, kOsAndroid,
-     FEATURE_WITH_PARAMS_VALUE_TYPE(chrome::android::kUseNewDoodleApi,
-                                    kUseNewDoodleApiVariations,
-                                    chrome::android::kUseNewDoodleApi.name)},
-#endif  // OS_ANDROID
-
     {"memory-ablation", flag_descriptions::kMemoryAblationName,
      flag_descriptions::kMemoryAblationDescription, kOsAll,
      FEATURE_WITH_PARAMS_VALUE_TYPE(kMemoryAblationFeature,

chrome/browser/android/chrome_feature_list.cc

@@ -230,9 +230,6 @@ const base::Feature kSpecialLocaleWrapper{"SpecialLocaleWrapper",
 const base::Feature kTabReparenting{"TabReparenting",
                                     base::FEATURE_ENABLED_BY_DEFAULT};
 
-const base::Feature kUseNewDoodleApi{"UseNewDoodleApi",
-                                     base::FEATURE_DISABLED_BY_DEFAULT};
-
 const base::Feature kUserMediaScreenCapturing{
     "UserMediaScreenCapturing", base::FEATURE_DISABLED_BY_DEFAULT};
 

chrome/browser/android/chrome_feature_list.h

@@ -50,7 +50,6 @@ extern const base::Feature kPhysicalWebSharing;
 extern const base::Feature kSpecialLocaleFeature;
 extern const base::Feature kSpecialLocaleWrapper;
 extern const base::Feature kTabReparenting;
-extern const base::Feature kUseNewDoodleApi;
 extern const base::Feature kUserMediaScreenCapturing;
 extern const base::Feature kVideoPersistence;
 extern const base::Feature kVrBrowsingFeedback;

chrome/browser/android/logo_bridge.cc

@@ -10,13 +10,8 @@
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
-#include "base/android/scoped_java_ref.h"
-#include "base/bind.h"
-#include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
-#include "chrome/browser/android/chrome_feature_list.h"
 #include "chrome/browser/android/logo_service.h"
-#include "chrome/browser/doodle/doodle_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_android.h"
 #include "components/search_provider_logos/logo_tracker.h"
@@ -27,7 +22,6 @@
 #include "net/url_request/url_request_status.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/android/java_bitmap.h"
-#include "ui/gfx/image/image.h"
 #include "url/gurl.h"
 
 using base::android::ConvertJavaStringToUTF8;
@@ -188,18 +182,11 @@ static jlong Init(JNIEnv* env,
 
 LogoBridge::LogoBridge(jobject j_profile)
     : logo_service_(nullptr),
-      doodle_service_(nullptr),
-      doodle_observer_(this),
       weak_ptr_factory_(this) {
   Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
   DCHECK(profile);
 
-  if (base::FeatureList::IsEnabled(chrome::android::kUseNewDoodleApi)) {
-    doodle_service_ = DoodleServiceFactory::GetForProfile(profile);
-    doodle_observer_.Add(doodle_service_);
-  } else {
-    logo_service_ = LogoServiceFactory::GetForProfile(profile);
-  }
+  logo_service_ = LogoServiceFactory::GetForProfile(profile);
 
   animated_logo_fetcher_ = base::MakeUnique<AnimatedLogoFetcher>(
       profile->GetRequestContext());
@@ -214,23 +201,10 @@ void LogoBridge::Destroy(JNIEnv* env, const JavaParamRef<jobject>& obj) {
 void LogoBridge::GetCurrentLogo(JNIEnv* env,
                                 const JavaParamRef<jobject>& obj,
                                 const JavaParamRef<jobject>& j_logo_observer) {
-  if (doodle_service_) {
-    j_logo_observer_.Reset(j_logo_observer);
-
-    // Hand out any current cached config.
-    if (doodle_service_->config().has_value()) {
-      FetchDoodleImage(doodle_service_->config().value(), /*from_cache=*/true);
-    }
-    // Also request a refresh, in case something changed. Depending on whether a
-    // newer config was available, either |OnDoodleConfigUpdated| or
-    // |OnDoodleConfigRevalidated| are called.
-    doodle_service_->Refresh();
-  } else {
-    // |observer| is deleted in LogoObserverAndroid::OnObserverRemoved().
-    LogoObserverAndroid* observer = new LogoObserverAndroid(
-        weak_ptr_factory_.GetWeakPtr(), env, j_logo_observer);
-    logo_service_->GetLogo(observer);
-  }
+  // |observer| is deleted in LogoObserverAndroid::OnObserverRemoved().
+  LogoObserverAndroid* observer = new LogoObserverAndroid(
+      weak_ptr_factory_.GetWeakPtr(), env, j_logo_observer);
+  logo_service_->GetLogo(observer);
 }
 
 void LogoBridge::GetAnimatedLogo(JNIEnv* env,
@@ -241,70 +215,6 @@ void LogoBridge::GetAnimatedLogo(JNIEnv* env,
   animated_logo_fetcher_->Start(env, url, j_callback);
 }
 
-void LogoBridge::OnDoodleConfigRevalidated(bool from_cache) {
-  if (j_logo_observer_.is_null()) {
-    return;
-  }
-  // If an existing config got re-validated, there's nothing to do - the UI is
-  // already in the correct state. However, we do tell the UI when we validate
-  // that there really isn't a Doodle. This is needed for metrics tracking.
-  if (!doodle_service_->config().has_value()) {
-    NotifyNoLogoAvailable(from_cache);
-  }
-}
-
-void LogoBridge::OnDoodleConfigUpdated(
-    const base::Optional<doodle::DoodleConfig>& maybe_doodle_config) {
-  if (j_logo_observer_.is_null()) {
-    return;
-  }
-  if (!maybe_doodle_config.has_value()) {
-    NotifyNoLogoAvailable(/*from_cache=*/false);
-    return;
-  }
-  FetchDoodleImage(maybe_doodle_config.value(), /*from_cache=*/false);
-}
-
-void LogoBridge::NotifyNoLogoAvailable(bool from_cache) {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_LogoObserver_onLogoAvailable(env, j_logo_observer_,
-                                    ScopedJavaLocalRef<jobject>(), from_cache);
-}
-
-void LogoBridge::FetchDoodleImage(const doodle::DoodleConfig& doodle_config,
-                                  bool from_cache) {
-  DCHECK(!j_logo_observer_.is_null());
-
-  // If there is a CTA image, that means the main image is animated. We show the
-  // non-animated CTA image first, and load the animated one only when the
-  // user requests it.
-  bool has_cta = doodle_config.large_cta_image.has_value();
-  const GURL& animated_image_url =
-      has_cta ? doodle_config.large_image.url : GURL::EmptyGURL();
-  const GURL& on_click_url = doodle_config.target_url;
-  const std::string& alt_text = doodle_config.alt_text;
-  doodle_service_->GetImage(
-      base::Bind(&LogoBridge::DoodleImageFetched, base::Unretained(this),
-                 from_cache, on_click_url, alt_text, animated_image_url));
-}
-
-void LogoBridge::DoodleImageFetched(bool config_from_cache,
-                                    const GURL& on_click_url,
-                                    const std::string& alt_text,
-                                    const GURL& animated_image_url,
-                                    const gfx::Image& image) {
-  JNIEnv* env = base::android::AttachCurrentThread();
-
-  ScopedJavaLocalRef<jobject> j_logo;
-  if (!image.IsEmpty()) {
-    j_logo = MakeJavaLogo(env, image.ToSkBitmap(), on_click_url, alt_text,
-                          animated_image_url);
-  }
-
-  Java_LogoObserver_onLogoAvailable(env, j_logo_observer_, j_logo,
-                                    config_from_cache);
-}
-
 // static
 bool RegisterLogoBridge(JNIEnv* env) {
   return RegisterNativesImpl(env);

chrome/browser/android/logo_bridge.h

@@ -12,23 +12,12 @@
 #include "base/android/scoped_java_ref.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "base/optional.h"
-#include "base/scoped_observer.h"
-#include "components/doodle/doodle_service.h"
 
 class LogoService;
 
-namespace doodle {
-class DoodleService;
-}  // namespace doodle
-
-namespace gfx {
-class Image;
-}  // namespace gfx
-
 // The C++ counterpart to LogoBridge.java. Enables Java code to access the
 // default search provider's logo.
-class LogoBridge : public doodle::DoodleService::Observer {
+class LogoBridge {
  public:
   explicit LogoBridge(jobject j_profile);
   void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
@@ -59,29 +48,8 @@ class LogoBridge : public doodle::DoodleService::Observer {
 
   virtual ~LogoBridge();
 
-  // doodle::DoodleService::Observer implementation.
-  void OnDoodleConfigRevalidated(bool from_cache) override;
-  void OnDoodleConfigUpdated(
-      const base::Optional<doodle::DoodleConfig>& maybe_doodle_config) override;
-
-  void NotifyNoLogoAvailable(bool from_cache);
-  void FetchDoodleImage(const doodle::DoodleConfig& doodle_config,
-                        bool from_cache);
-  void DoodleImageFetched(bool config_from_cache,
-                          const GURL& on_click_url,
-                          const std::string& alt_text,
-                          const GURL& animated_image_url,
-                          const gfx::Image& image);
-
-  // Only valid if UseNewDoodleApi is disabled.
   LogoService* logo_service_;
 
-  // Only valid if UseNewDoodleApi is enabled.
-  doodle::DoodleService* doodle_service_;
-  base::android::ScopedJavaGlobalRef<jobject> j_logo_observer_;
-  ScopedObserver<doodle::DoodleService, doodle::DoodleService::Observer>
-      doodle_observer_;
-
   std::unique_ptr<AnimatedLogoFetcher> animated_logo_fetcher_;
 
   base::WeakPtrFactory<LogoBridge> weak_ptr_factory_;

chrome/browser/doodle/OWNERS

-file://components/doodle/OWNERS

chrome/browser/doodle/doodle_service_factory.cc

-// 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.
-
-#include "chrome/browser/doodle/doodle_service_factory.h"
-
-#include <memory>
-#include <utility>
-
-#include "base/memory/ptr_util.h"
-#include "base/metrics/field_trial_params.h"
-#include "base/time/default_clock.h"
-#include "base/time/default_tick_clock.h"
-#include "base/timer/timer.h"
-#include "chrome/browser/google/google_url_tracker_factory.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/search/suggestions/image_decoder_impl.h"
-#include "components/doodle/doodle_fetcher.h"
-#include "components/doodle/doodle_fetcher_impl.h"
-#include "components/doodle/doodle_service.h"
-#include "components/image_fetcher/core/image_fetcher_impl.h"
-#include "components/keyed_service/content/browser_context_dependency_manager.h"
-#include "components/prefs/pref_service.h"
-#include "components/safe_json/safe_json_parser.h"
-
-#if defined(OS_ANDROID)
-#include "chrome/browser/android/chrome_feature_list.h"
-#endif
-
-#if defined(OS_ANDROID)
-namespace {
-const char kOverrideUrlParam[] = "doodle_override_url";
-}  // namespace
-#endif
-
-// static
-DoodleServiceFactory* DoodleServiceFactory::GetInstance() {
-  return base::Singleton<DoodleServiceFactory>::get();
-}
-
-// static
-doodle::DoodleService* DoodleServiceFactory::GetForProfile(Profile* profile) {
-  return static_cast<doodle::DoodleService*>(
-      GetInstance()->GetServiceForBrowserContext(profile, /*create=*/true));
-}
-
-DoodleServiceFactory::DoodleServiceFactory()
-    : BrowserContextKeyedServiceFactory(
-          "DoodleService",
-          BrowserContextDependencyManager::GetInstance()) {
-  DependsOn(GoogleURLTrackerFactory::GetInstance());
-}
-
-DoodleServiceFactory::~DoodleServiceFactory() = default;
-
-KeyedService* DoodleServiceFactory::BuildServiceInstanceFor(
-    content::BrowserContext* context) const {
-  Profile* profile = static_cast<Profile*>(context);
-  // We don't show doodles in incognito profiles (for now?).
-  DCHECK(!profile->IsOffTheRecord());
-
-  bool use_gray_background = false;
-  base::Optional<std::string> override_url;
-
-#if defined(OS_ANDROID)
-  DCHECK(base::FeatureList::IsEnabled(chrome::android::kUseNewDoodleApi));
-
-  use_gray_background =
-      !base::FeatureList::IsEnabled(chrome::android::kChromeHomeFeature);
-
-  std::string override_url_str = base::GetFieldTrialParamValueByFeature(
-      chrome::android::kUseNewDoodleApi, kOverrideUrlParam);
-  // GetFieldTrialParamValueByFeature returns an empty string if the param is
-  // not set.
-  if (!override_url_str.empty()) {
-    override_url = override_url_str;
-  }
-#endif
-
-  auto fetcher = base::MakeUnique<doodle::DoodleFetcherImpl>(
-      profile->GetRequestContext(),
-      GoogleURLTrackerFactory::GetForProfile(profile),
-      base::Bind(&safe_json::SafeJsonParser::Parse), use_gray_background,
-      override_url);
-  return new doodle::DoodleService(
-      profile->GetPrefs(), std::move(fetcher),
-      base::MakeUnique<base::OneShotTimer>(),
-      base::MakeUnique<base::DefaultClock>(),
-      base::MakeUnique<base::DefaultTickClock>(),
-      /*override_min_refresh_interval=*/base::nullopt,
-      base::MakeUnique<image_fetcher::ImageFetcherImpl>(
-          base::MakeUnique<suggestions::ImageDecoderImpl>(),
-          profile->GetRequestContext()));
-}

chrome/browser/doodle/doodle_service_factory.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 CHROME_BROWSER_DOODLE_DOODLE_SERVICE_FACTORY_H_
-#define CHROME_BROWSER_DOODLE_DOODLE_SERVICE_FACTORY_H_
-
-#include "base/macros.h"
-#include "base/memory/singleton.h"
-#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
-
-class Profile;
-
-namespace doodle {
-class DoodleService;
-}  // namespace doodle
-
-class DoodleServiceFactory : public BrowserContextKeyedServiceFactory {
- public:
-  static DoodleServiceFactory* GetInstance();
-  static doodle::DoodleService* GetForProfile(Profile* profile);
-
- private:
-  friend struct base::DefaultSingletonTraits<DoodleServiceFactory>;
-
-  DoodleServiceFactory();
-  ~DoodleServiceFactory() override;
-
-  // BrowserContextKeyedServiceFactory implementation.
-  KeyedService* BuildServiceInstanceFor(
-      content::BrowserContext* context) const override;
-
-  DISALLOW_COPY_AND_ASSIGN(DoodleServiceFactory);
-};
-
-#endif  // CHROME_BROWSER_DOODLE_DOODLE_SERVICE_FACTORY_H_