Commit 47c18b77 authored by ianwen's avatar ianwen Committed by Commit bot

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}
parent 23fa8b24
sky@chromium.org sky@chromium.org
per-file enhanced_bookmarks_features*=wittman@chromium.org
\ No newline at end of file
...@@ -5,19 +5,8 @@ ...@@ -5,19 +5,8 @@
#include "chrome/browser/bookmarks/enhanced_bookmarks_features.h" #include "chrome/browser/bookmarks/enhanced_bookmarks_features.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/metrics/histogram.h"
#include "base/prefs/pref_service.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/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 "components/variations/variations_associated_data.h"
#include "extensions/common/features/feature.h" #include "extensions/common/features/feature.h"
#include "extensions/common/features/feature_provider.h" #include "extensions/common/features/feature_provider.h"
...@@ -37,17 +26,8 @@ bool GetBookmarksExperimentExtensionID(std::string* extension_id) { ...@@ -37,17 +26,8 @@ bool GetBookmarksExperimentExtensionID(std::string* extension_id) {
if (extension_id->empty()) if (extension_id->empty())
return false; 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) #if defined(OS_ANDROID)
return base::android::BuildInfo::GetInstance()->sdk_int() > return true;
base::android::SdkVersion::SDK_VERSION_ICE_CREAM_SANDWICH_MR1;
#else #else
const extensions::FeatureProvider* feature_provider = const extensions::FeatureProvider* feature_provider =
extensions::FeatureProvider::GetPermissionFeatures(); extensions::FeatureProvider::GetPermissionFeatures();
...@@ -73,10 +53,33 @@ bool IsEnhancedBookmarkImageFetchingEnabled(const PrefService* user_prefs) { ...@@ -73,10 +53,33 @@ bool IsEnhancedBookmarkImageFetchingEnabled(const PrefService* user_prefs) {
bool IsEnhancedBookmarksEnabled() { bool IsEnhancedBookmarksEnabled() {
std::string extension_id; std::string extension_id;
return GetBookmarksExperimentExtensionID(&extension_id); return IsEnhancedBookmarksEnabled(&extension_id);
} }
#endif #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() { bool IsEnableDomDistillerSet() {
if (base::CommandLine::ForCurrentProcess()->HasSwitch( if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableDomDistiller)) { switches::kEnableDomDistiller)) {
......
...@@ -11,10 +11,6 @@ ...@@ -11,10 +11,6 @@
class PrefService; 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) #if defined(OS_ANDROID)
// Returns true if enhanced bookmark salient image prefetching is enabled. // Returns true if enhanced bookmark salient image prefetching is enabled.
// This can be controlled by field trial. // This can be controlled by field trial.
...@@ -22,9 +18,14 @@ bool IsEnhancedBookmarkImageFetchingEnabled(const PrefService* user_prefs); ...@@ -22,9 +18,14 @@ bool IsEnhancedBookmarkImageFetchingEnabled(const PrefService* user_prefs);
// Returns true if enhanced bookmarks is enabled. // Returns true if enhanced bookmarks is enabled.
bool IsEnhancedBookmarksEnabled(); bool IsEnhancedBookmarksEnabled();
#endif #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. // Returns true when flag enable-dom-distiller is set or enabled from Finch.
bool IsEnableDomDistillerSet(); bool IsEnableDomDistillerSet();
......
...@@ -67,7 +67,7 @@ void ExternalComponentLoader::StartLoading() { ...@@ -67,7 +67,7 @@ void ExternalComponentLoader::StartLoading() {
{ {
std::string extension_id; std::string extension_id;
if (GetBookmarksExperimentExtensionID(&extension_id)) { if (IsEnhancedBookmarksEnabled(&extension_id)) {
prefs_->SetString(extension_id + ".external_update_url", prefs_->SetString(extension_id + ".external_update_url",
extension_urls::GetWebstoreUpdateUrl().spec()); extension_urls::GetWebstoreUpdateUrl().spec());
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment