Skip to content

Commit

Permalink
[Icons NTP] Using flag to enable large icons on the NTP.
Browse files Browse the repository at this point in the history
A follow-up to crrev.com/1026053003: We need to pass the flag for
enabling icon NTP to the 3 end points that use them. One endpoint
(searchbox_extetnsion.cc) is in the Renderer process, and the flag (as
command line) needs to be propagated. To obey existing dependencies,
the flags kDisableIconNtp and kEnableIconNtp need to be moved to
ui_base_witches.*. As for enabling using variation, the code is kept
near the end points to avoid adding dependency to ui_base.

BUG=467712

Review URL: https://codereview.chromium.org/1043753002

Cr-Commit-Position: refs/heads/master@{#323570}
  • Loading branch information
samuelhuang authored and Commit bot committed Apr 2, 2015
1 parent 0d468f8 commit ab02317
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 17 deletions.
18 changes: 15 additions & 3 deletions chrome/browser/favicon/favicon_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "chrome/browser/favicon/favicon_tab_helper.h"

#include "base/command_line.h"
#include "base/metrics/field_trial.h"
#include "base/strings/string_util.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/chrome_notification_types.h"
Expand All @@ -20,7 +22,6 @@
#include "components/favicon/core/favicon_service.h"
#include "components/favicon_base/favicon_types.h"
#include "components/history/core/browser/history_service.h"
#include "components/variations/variations_associated_data.h"
#include "content/public/browser/favicon_status.h"
#include "content/public/browser/invalidate_type.h"
#include "content/public/browser/navigation_controller.h"
Expand All @@ -31,6 +32,7 @@
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/common/favicon_url.h"
#include "ui/base/ui_base_switches.h"
#include "ui/gfx/codec/png_codec.h"
#include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_skia.h"
Expand All @@ -45,9 +47,19 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(FaviconTabHelper);

namespace {

// Returns whether icon NTP is enabled.
// Returns whether icon NTP is enabled by experiment.
// TODO(huangs): Remove all 3 copies of this routine once Icon NTP launches.
bool IsIconNTPEnabled() {
return variations::GetVariationParamValue("IconNTP", "state") == "enabled";
// Note: It's important to query the field trial state first, to ensure that
// UMA reports the correct group.
const std::string group_name = base::FieldTrialList::FindFullName("IconNTP");
using base::CommandLine;
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableIconNtp))
return false;
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableIconNtp))
return true;

return StartsWithASCII(group_name, "Enabled", true);
}

#if defined(OS_ANDROID) || defined(OS_IOS)
Expand Down
17 changes: 14 additions & 3 deletions chrome/browser/search/local_ntp_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "chrome/browser/search/local_ntp_source.h"

#include "base/command_line.h"
#include "base/json/json_string_value_serializer.h"
#include "base/logging.h"
#include "base/memory/ref_counted_memory.h"
Expand All @@ -20,12 +21,12 @@
#include "chrome/grit/generated_resources.h"
#include "components/search_engines/template_url_prepopulate_data.h"
#include "components/search_engines/template_url_service.h"
#include "components/variations/variations_associated_data.h"
#include "grit/browser_resources.h"
#include "grit/theme_resources.h"
#include "net/url_request/url_request.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/base/ui_base_switches.h"
#include "ui/base/webui/jstemplate_builder.h"
#include "ui/base/webui/web_ui_util.h"
#include "ui/resources/grit/ui_resources.h"
Expand Down Expand Up @@ -81,9 +82,19 @@ bool DefaultSearchProviderIsGoogle(Profile* profile) {
SEARCH_ENGINE_GOOGLE);
}

// Returns whether icon NTP is enabled.
// Returns whether icon NTP is enabled by experiment.
// TODO(huangs): Remove all 3 copies of this routine once Icon NTP launches.
bool IsIconNTPEnabled() {
return variations::GetVariationParamValue("IconNTP", "state") == "enabled";
// Note: It's important to query the field trial state first, to ensure that
// UMA reports the correct group.
const std::string group_name = base::FieldTrialList::FindFullName("IconNTP");
using base::CommandLine;
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableIconNtp))
return false;
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableIconNtp))
return true;

return StartsWithASCII(group_name, "Enabled", true);
}

// Returns whether we are in the Fast NTP experiment or not.
Expand Down
6 changes: 0 additions & 6 deletions chrome/common/chrome_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,6 @@ const char kDisableExtensionsFileAccessCheck[] =
const char kDisableExtensionsHttpThrottling[] =
"disable-extensions-http-throttling";

// Disables large icons on the New Tab page.
const char kDisableIconNtp[] = "disable-icon-ntp";

// Don't resolve hostnames to IPv6 addresses. This can be used when debugging
// issues relating to IPv6, but shouldn't otherwise be needed. Be sure to file
// bugs if something isn't working properly in the presence of IPv6. This flag
Expand Down Expand Up @@ -466,9 +463,6 @@ const char kEnableExtensionActivityLogTesting[] =
// crbug.com/142458 .
const char kEnableFastUnload[] = "enable-fast-unload";

// Enables large icons on the New Tab page.
const char kEnableIconNtp[] = "enable-icon-ntp";

// Enable opt-in for the collection of invalid TLS/SSL certificate chains.
const char kEnableInvalidCertCollection[] = "enable-invalid-cert-collection";

Expand Down
2 changes: 0 additions & 2 deletions chrome/common/chrome_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ extern const char kDisableDomainReliability[];
extern const char kDisableExtensionsFileAccessCheck[];
extern const char kDisableExtensionsHttpThrottling[];
extern const char kDisableExtensions[];
extern const char kDisableIconNtp[];
extern const char kDisableIPv6[];
extern const char kDisableJavaScriptHarmonyShipping[];
extern const char kDisableMinimizeOnSecondLauncherItemClick[];
Expand Down Expand Up @@ -135,7 +134,6 @@ extern const char kEnableExperimentalHotwordHardware[];
extern const char kEnableExtensionActivityLogging[];
extern const char kEnableExtensionActivityLogTesting[];
extern const char kEnableFastUnload[];
extern const char kEnableIconNtp[];
extern const char kEnableInvalidCertCollection[];
extern const char kEnableIPv6[];
extern const char kEnableLinkableEphemeralApps[];
Expand Down
18 changes: 15 additions & 3 deletions chrome/renderer/searchbox/searchbox_extension.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

#include "chrome/renderer/searchbox/searchbox_extension.h"

#include "base/command_line.h"
#include "base/i18n/rtl.h"
#include "base/json/string_escape.h"
#include "base/metrics/field_trial.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
Expand All @@ -16,14 +18,14 @@
#include "chrome/grit/renderer_resources.h"
#include "chrome/renderer/searchbox/searchbox.h"
#include "components/crx_file/id_util.h"
#include "components/variations/variations_associated_data.h"
#include "content/public/renderer/render_view.h"
#include "third_party/WebKit/public/platform/WebURLRequest.h"
#include "third_party/WebKit/public/web/WebDocument.h"
#include "third_party/WebKit/public/web/WebLocalFrame.h"
#include "third_party/WebKit/public/web/WebScriptSource.h"
#include "third_party/WebKit/public/web/WebView.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/base/ui_base_switches.h"
#include "ui/base/window_open_disposition.h"
#include "ui/events/keycodes/keyboard_codes.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -62,9 +64,19 @@ base::string16 V8ValueToUTF16(v8::Handle<v8::Value> v) {
return base::string16(reinterpret_cast<const base::char16*>(*s), s.length());
}

// Returns whether we should use large icons on NTP.
// Returns whether icon NTP is enabled by experiment.
// TODO(huangs): Remove all 3 copies of this routine once Icon NTP launches.
bool IsIconNTPEnabled() {
return variations::GetVariationParamValue("IconNTP", "state") == "enabled";
// Note: It's important to query the field trial state first, to ensure that
// UMA reports the correct group.
const std::string group_name = base::FieldTrialList::FindFullName("IconNTP");
using base::CommandLine;
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableIconNtp))
return false;
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableIconNtp))
return true;

return StartsWithASCII(group_name, "Enabled", true);
}

// Converts string16 to V8 String.
Expand Down
2 changes: 2 additions & 0 deletions content/browser/renderer_host/render_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1219,6 +1219,7 @@ void RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
switches::kDisableGpuVsync,
switches::kDisableLowResTiling,
switches::kDisableHistogramCustomizer,
switches::kDisableIconNtp,
switches::kDisableLCDText,
switches::kDisableLocalStorage,
switches::kDisableLogging,
Expand Down Expand Up @@ -1253,6 +1254,7 @@ void RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
switches::kEnableGPUClientLogging,
switches::kEnableGpuClientTracing,
switches::kEnableGPUServiceLogging,
switches::kEnableIconNtp,
switches::kEnableLinkDisambiguationPopup,
switches::kEnableLowResTiling,
switches::kEnableInbandTextTracks,
Expand Down
6 changes: 6 additions & 0 deletions ui/base/ui_base_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ const char kDisableRemoteCoreAnimation[] = "disable-remote-core-animation";
// Disables use of DWM composition for top level windows.
const char kDisableDwmComposition[] = "disable-dwm-composition";

// Disables large icons on the New Tab page.
const char kDisableIconNtp[] = "disable-icon-ntp";

// Disables an experimental focus manager to track text input clients.
const char kDisableTextInputFocusManager[] = "disable-text-input-focus-manager";

Expand All @@ -30,6 +33,9 @@ const char kDisableTouchEditing[] = "disable-touch-editing";
// Disables additional visual feedback to touch input.
const char kDisableTouchFeedback[] = "disable-touch-feedback";

// Enables large icons on the New Tab page.
const char kEnableIconNtp[] = "enable-icon-ntp";

// Enables a zoomed popup bubble that allows the user to select a link.
const char kEnableLinkDisambiguationPopup[] =
"enable-link-disambiguation-popup";
Expand Down
2 changes: 2 additions & 0 deletions ui/base/ui_base_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ UI_BASE_EXPORT extern const char kDisableRemoteCoreAnimation[];
#endif

UI_BASE_EXPORT extern const char kDisableDwmComposition[];
UI_BASE_EXPORT extern const char kDisableIconNtp[];
UI_BASE_EXPORT extern const char kDisableTextInputFocusManager[];
UI_BASE_EXPORT extern const char kDisableTouchAdjustment[];
UI_BASE_EXPORT extern const char kDisableTouchDragDrop[];
UI_BASE_EXPORT extern const char kDisableTouchEditing[];
UI_BASE_EXPORT extern const char kDisableTouchFeedback[];
UI_BASE_EXPORT extern const char kEnableIconNtp[];
UI_BASE_EXPORT extern const char kEnableLinkDisambiguationPopup[];
UI_BASE_EXPORT extern const char kEnableTextInputFocusManager[];
UI_BASE_EXPORT extern const char kEnableTouchDragDrop[];
Expand Down

0 comments on commit ab02317

Please sign in to comment.