Commit e792609b authored by Justin Donnelly's avatar Justin Donnelly Committed by Commit Bot

Make the new omnibox UI features dependent on the MD Refresh UI.

These features (tab switch suggestions, new answer layout and rich
entities) all work without the MD Refresh UI, but the layout looks
weird. It's not something we'd want to launch. So this change disables
them if the MD Refresh UI is not also enabled.

The intent in taking this approach (vs. just enabling the correct
combination of flags manually or via experiment config) is two-fold:
1. Ensure that when we start experiments on these, if the MD Refresh UI
   is turned off, these features will automatically be turned off as
   well for users in the experimental group.
2. Prevent people from enabling the new features without MD Refresh and
   then filing bugs for a configuration that we don't intend to support.

Change-Id: Ie29ba598ae04a4d5e14437d0dd8d5e189815ef40
Reviewed-on: https://chromium-review.googlesource.com/1103152Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568005}
parent afa62a08
...@@ -2516,16 +2516,22 @@ const char kOmniboxRichEntitySuggestionsName[] = ...@@ -2516,16 +2516,22 @@ const char kOmniboxRichEntitySuggestionsName[] =
"Omnibox rich entity suggestions"; "Omnibox rich entity suggestions";
const char kOmniboxRichEntitySuggestionsDescription[] = const char kOmniboxRichEntitySuggestionsDescription[] =
"Display entity suggestions using images and an enhanced layout; showing " "Display entity suggestions using images and an enhanced layout; showing "
"more context and descriptive text about the entity"; "more context and descriptive text about the entity. Has no effect unless "
"either the #upcoming-ui-features flag is Enabled or the #top-chrome-md "
"flag is set to Refresh or Touchable Refresh.";
const char kOmniboxNewAnswerLayoutName[] = "Omnibox new answer layout"; const char kOmniboxNewAnswerLayoutName[] = "Omnibox new answer layout";
const char kOmniboxNewAnswerLayoutDescription[] = const char kOmniboxNewAnswerLayoutDescription[] =
"Display answers using an enhanced layout with larger images"; "Display answers using an enhanced layout with larger images. Has no "
"effect unless either the #upcoming-ui-features flag is Enabled or the "
"#top-chrome-md flag is set to Refresh or Touchable Refresh.";
const char kOmniboxTabSwitchSuggestionsName[] = const char kOmniboxTabSwitchSuggestionsName[] =
"Omnibox tab switch suggestions"; "Omnibox tab switch suggestions";
const char kOmniboxTabSwitchSuggestionsDescription[] = const char kOmniboxTabSwitchSuggestionsDescription[] =
"Enable suggestions for switching to open tabs within the Omnibox."; "Enable suggestions for switching to open tabs within the Omnibox. "
"Has no effect unless either the #upcoming-ui-features flag is Enabled or "
"the #top-chrome-md flag is set to Refresh or Touchable Refresh.";
const char kOmniboxTailSuggestionsName[] = "Omnibox tail suggestions"; const char kOmniboxTailSuggestionsName[] = "Omnibox tail suggestions";
const char kOmniboxTailSuggestionsDescription[] = const char kOmniboxTailSuggestionsDescription[] =
......
...@@ -269,7 +269,7 @@ void OmniboxMatchCellView::OnMatchUpdate(const OmniboxResultView* result_view, ...@@ -269,7 +269,7 @@ void OmniboxMatchCellView::OnMatchUpdate(const OmniboxResultView* result_view,
is_old_style_answer_ = !!match.answer; is_old_style_answer_ = !!match.answer;
is_rich_suggestion_ = is_rich_suggestion_ =
(OmniboxFieldTrial::IsNewAnswerLayoutEnabled() && !!match.answer) || (OmniboxFieldTrial::IsNewAnswerLayoutEnabled() && !!match.answer) ||
(base::FeatureList::IsEnabled(omnibox::kOmniboxRichEntitySuggestions) && (OmniboxFieldTrial::IsRichEntitySuggestionsEnabled() &&
!match.image_url.empty()); !match.image_url.empty());
is_search_type_ = AutocompleteMatch::IsSearchType(match.type); is_search_type_ = AutocompleteMatch::IsSearchType(match.type);
has_tab_match_ = match.has_tab_match; has_tab_match_ = match.has_tab_match;
......
...@@ -679,13 +679,23 @@ OmniboxFieldTrial::GetEmphasizeTitlesConditionForInput( ...@@ -679,13 +679,23 @@ OmniboxFieldTrial::GetEmphasizeTitlesConditionForInput(
return static_cast<EmphasizeTitlesCondition>(value); return static_cast<EmphasizeTitlesCondition>(value);
} }
bool OmniboxFieldTrial::IsRichEntitySuggestionsEnabled() {
return base::FeatureList::IsEnabled(omnibox::kOmniboxRichEntitySuggestions) &&
ui::MaterialDesignController::is_mode_initialized() &&
ui::MaterialDesignController::IsRefreshUi();
}
bool OmniboxFieldTrial::IsNewAnswerLayoutEnabled() { bool OmniboxFieldTrial::IsNewAnswerLayoutEnabled() {
return base::FeatureList::IsEnabled(omnibox::kOmniboxNewAnswerLayout) || return (base::FeatureList::IsEnabled(omnibox::kOmniboxNewAnswerLayout) &&
ui::MaterialDesignController::is_mode_initialized() &&
ui::MaterialDesignController::IsRefreshUi()) ||
base::FeatureList::IsEnabled(features::kExperimentalUi); base::FeatureList::IsEnabled(features::kExperimentalUi);
} }
bool OmniboxFieldTrial::IsTabSwitchSuggestionsEnabled() { bool OmniboxFieldTrial::IsTabSwitchSuggestionsEnabled() {
return base::FeatureList::IsEnabled(omnibox::kOmniboxTabSwitchSuggestions) || return (base::FeatureList::IsEnabled(omnibox::kOmniboxTabSwitchSuggestions) &&
ui::MaterialDesignController::is_mode_initialized() &&
ui::MaterialDesignController::IsRefreshUi()) ||
base::FeatureList::IsEnabled(features::kExperimentalUi); base::FeatureList::IsEnabled(features::kExperimentalUi);
} }
......
...@@ -408,12 +408,15 @@ class OmniboxFieldTrial { ...@@ -408,12 +408,15 @@ class OmniboxFieldTrial {
// --------------------------------------------------------- // ---------------------------------------------------------
// For tab switch suggestions related experiments. // For tab switch suggestions related experiments.
// Returns true if either the new answer layout flag or the // Returns true if the rich entities flag and the refresh UI is enabled.
// #upcoming-ui-features flag is enabled. static bool IsRichEntitySuggestionsEnabled();
// Returns true if either (the new answer layout flag and the refresh UI) or
// the #upcoming-ui-features flag is enabled.
static bool IsNewAnswerLayoutEnabled(); static bool IsNewAnswerLayoutEnabled();
// Returns true if either the tab switch suggestions flag or the // Returns true if either (the tab switch suggestions flag and the refresh UI)
// #upcoming-ui-features flag is enabled. // or the #upcoming-ui-features flag is enabled.
static bool IsTabSwitchSuggestionsEnabled(); static bool IsTabSwitchSuggestionsEnabled();
// Returns true if either the steady-state elision flag or the // Returns true if either the steady-state elision flag or the
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/values.h" #include "base/values.h"
#include "components/omnibox/browser/omnibox_field_trial.h" #include "components/omnibox/browser/omnibox_field_trial.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/ui_base_features.h"
namespace { namespace {
...@@ -265,7 +266,7 @@ TEST(SuggestionAnswerTest, AddImageURLsTo) { ...@@ -265,7 +266,7 @@ TEST(SuggestionAnswerTest, AddImageURLsTo) {
{ {
base::test::ScopedFeatureList feature_list; base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(omnibox::kOmniboxNewAnswerLayout); feature_list.InitAndEnableFeature(features::kExperimentalUi);
json = json =
"{ \"i\": { \"d\": \"https://gstatic.com/foo.png\", \"t\": 3 }," "{ \"i\": { \"d\": \"https://gstatic.com/foo.png\", \"t\": 3 },"
" \"l\" : [" " \"l\" : ["
......
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