From 47c18b773aa75bdae2f4b39750534119bc3055ab Mon Sep 17 00:00:00 2001 From: ianwen Date: Thu, 19 Mar 2015 14:03:43 -0700 Subject: [PATCH] Force Enhanced Bookmark to be enabled if command line flags are set true kEnhancedBookmarksExperiment is used by android intrumentation tests to force enhanced bookmark feature to be enabled. https://codereview.chromium.org/1009673002 introduced a regression that the even if the flag is set to be true, it does not take effect. This CL fixes it. BUG=468106 Review URL: https://codereview.chromium.org/1017913002 Cr-Commit-Position: refs/heads/master@{#321429} --- chrome/browser/bookmarks/OWNERS | 2 + .../bookmarks/enhanced_bookmarks_features.cc | 47 ++++++++++++---------- .../bookmarks/enhanced_bookmarks_features.h | 11 ++--- .../extensions/external_component_loader.cc | 2 +- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/chrome/browser/bookmarks/OWNERS b/chrome/browser/bookmarks/OWNERS index 90b3e809ffe7..5d705235017b 100644 --- a/chrome/browser/bookmarks/OWNERS +++ b/chrome/browser/bookmarks/OWNERS @@ -1 +1,3 @@ sky@chromium.org + +per-file enhanced_bookmarks_features*=wittman@chromium.org \ No newline at end of file diff --git a/chrome/browser/bookmarks/enhanced_bookmarks_features.cc b/chrome/browser/bookmarks/enhanced_bookmarks_features.cc index 9f681c6b3876..5b7b38a0ad83 100644 --- a/chrome/browser/bookmarks/enhanced_bookmarks_features.cc +++ b/chrome/browser/bookmarks/enhanced_bookmarks_features.cc @@ -5,19 +5,8 @@ #include "chrome/browser/bookmarks/enhanced_bookmarks_features.h" #include "base/command_line.h" -#include "base/metrics/histogram.h" #include "base/prefs/pref_service.h" -#include "base/prefs/scoped_user_pref_update.h" -#include "base/sha1.h" -#include "base/strings/string_number_conversions.h" -#include "chrome/browser/browser_process.h" -#include "chrome/browser/flags_storage.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/signin/signin_manager_factory.h" #include "chrome/common/chrome_switches.h" -#include "chrome/common/pref_names.h" -#include "components/signin/core/browser/signin_manager.h" -#include "components/sync_driver/pref_names.h" #include "components/variations/variations_associated_data.h" #include "extensions/common/features/feature.h" #include "extensions/common/features/feature_provider.h" @@ -37,17 +26,8 @@ bool GetBookmarksExperimentExtensionID(std::string* extension_id) { if (extension_id->empty()) return false; - // kEnhancedBookmarksExperiment flag could have values "", "1" and "0". - // "0" - user opted out. - bool opt_out = base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( - switches::kEnhancedBookmarksExperiment) == "0"; - - if (opt_out) - return false; - #if defined(OS_ANDROID) - return base::android::BuildInfo::GetInstance()->sdk_int() > - base::android::SdkVersion::SDK_VERSION_ICE_CREAM_SANDWICH_MR1; + return true; #else const extensions::FeatureProvider* feature_provider = extensions::FeatureProvider::GetPermissionFeatures(); @@ -73,10 +53,33 @@ bool IsEnhancedBookmarkImageFetchingEnabled(const PrefService* user_prefs) { bool IsEnhancedBookmarksEnabled() { std::string extension_id; - return GetBookmarksExperimentExtensionID(&extension_id); + return IsEnhancedBookmarksEnabled(&extension_id); } #endif +bool IsEnhancedBookmarksEnabled(std::string* extension_id) { + // kEnhancedBookmarksExperiment flag could have values "", "1" and "0". + // "0" - user opted out. + bool opt_out = base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( + switches::kEnhancedBookmarksExperiment) == "0"; + +#if defined(OS_ANDROID) + opt_out |= base::android::BuildInfo::GetInstance()->sdk_int() < + base::android::SdkVersion::SDK_VERSION_ICE_CREAM_SANDWICH_MR1; + + // Android tests use command line flag to force enhanced bookmark to be on. + bool opt_in = base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( + switches::kEnhancedBookmarksExperiment) == "1"; + if (opt_in) + return true; +#endif + + if (opt_out) + return false; + + return GetBookmarksExperimentExtensionID(extension_id); +} + bool IsEnableDomDistillerSet() { if (base::CommandLine::ForCurrentProcess()->HasSwitch( switches::kEnableDomDistiller)) { diff --git a/chrome/browser/bookmarks/enhanced_bookmarks_features.h b/chrome/browser/bookmarks/enhanced_bookmarks_features.h index 8e6a98701678..84183122ac2f 100644 --- a/chrome/browser/bookmarks/enhanced_bookmarks_features.h +++ b/chrome/browser/bookmarks/enhanced_bookmarks_features.h @@ -11,10 +11,6 @@ class PrefService; -// Returns true and sets |extension_id| if enhanced bookmarks experiment is -// enabled. Returns false if no bookmark experiment or extension id is empty. -bool GetBookmarksExperimentExtensionID(std::string* extension_id); - #if defined(OS_ANDROID) // Returns true if enhanced bookmark salient image prefetching is enabled. // This can be controlled by field trial. @@ -22,9 +18,14 @@ bool IsEnhancedBookmarkImageFetchingEnabled(const PrefService* user_prefs); // Returns true if enhanced bookmarks is enabled. bool IsEnhancedBookmarksEnabled(); - #endif +// Returns true and sets |extension_id| if enhanced bookmarks experiment is +// enabled. Returns false if no bookmark experiment or extension id is empty. +// Besides, this method takes commandline flags into account before trying +// to get an extension_id. +bool IsEnhancedBookmarksEnabled(std::string* extension_id); + // Returns true when flag enable-dom-distiller is set or enabled from Finch. bool IsEnableDomDistillerSet(); diff --git a/chrome/browser/extensions/external_component_loader.cc b/chrome/browser/extensions/external_component_loader.cc index 7d04357dddc3..e242332cf3d5 100644 --- a/chrome/browser/extensions/external_component_loader.cc +++ b/chrome/browser/extensions/external_component_loader.cc @@ -67,7 +67,7 @@ void ExternalComponentLoader::StartLoading() { { std::string extension_id; - if (GetBookmarksExperimentExtensionID(&extension_id)) { + if (IsEnhancedBookmarksEnabled(&extension_id)) { prefs_->SetString(extension_id + ".external_update_url", extension_urls::GetWebstoreUpdateUrl().spec()); } -- 2.11.4.GIT