Commit 103f7e6b authored by Emily Stark's avatar Emily Stark Committed by Commit Bot

Revert "Clean up steady-state scheme/trivial subdomains elision flags"

This reverts commit 65ac9625.

Reason for revert: Something got messed up in a rebase somehow and this ended up accidentally reverting parts of https://chromium-review.googlesource.com/c/chromium/src/+/2283655/

Original change's description:
> Clean up steady-state scheme/trivial subdomains elision flags
> 
> These flags are enabled by default and expired since M76.
> 
> I'm cleaning these up now because I'm starting to think about
> uneliding scheme/trivial subdomains on hover (for the omnibox UI
> simplified domain field trials), and it'll be simpler code if we don't
> have to reason about these expired flags in both enabled and disabled
> states.
> 
> Bug: 797354
> Change-Id: I766a30c2f886ae1d651c2bf8eed0bc094664bde7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2277880
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Commit-Queue: Emily Stark <estark@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#785962}

TBR=msw@chromium.org,tommycli@chromium.org,tedchoc@chromium.org,estark@chromium.org

Change-Id: I608b380c23af2211019b6fac2988a13da25f4d26
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 797354
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2285514Reviewed-by: default avatarEmily Stark <estark@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786020}
parent 54b4106d
......@@ -18,6 +18,7 @@ import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
import org.chromium.chrome.browser.dom_distiller.DomDistillerTabUtils;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.native_page.NativePageFactory;
import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
......@@ -425,6 +426,27 @@ public class LocationBarModel implements ToolbarDataProvider, ToolbarCommonPrope
return R.color.locationbar_status_preview_color;
}
if (!hasTab() || isUsingBrandColor()
|| ChromeFeatureList.isEnabled(
ChromeFeatureList.OMNIBOX_HIDE_SCHEME_IN_STEADY_STATE)
|| ChromeFeatureList.isEnabled(
ChromeFeatureList.OMNIBOX_HIDE_TRIVIAL_SUBDOMAINS_IN_STEADY_STATE)) {
// For theme colors which are not dark and are also not
// light enough to warrant an opaque URL bar, use dark
// icons.
return ToolbarColors.getThemedToolbarIconTintRes(false);
}
// TODO(https://crbug.com/940134): Change the color here and also #needLightIcon logic.
// For the default toolbar color, use a green or red icon.
if (securityLevel == ConnectionSecurityLevel.DANGEROUS) {
return R.color.google_red_600;
}
if (securityLevel == ConnectionSecurityLevel.SECURE) {
return R.color.google_green_600;
}
return ToolbarColors.getThemedToolbarIconTintRes(false);
}
......
......@@ -3559,6 +3559,17 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kOmniboxUIElideToRegistrableDomainDescription,
kOsDesktop, FEATURE_VALUE_TYPE(omnibox::kElideToRegistrableDomain)},
{"omnibox-ui-hide-steady-state-url-scheme",
flag_descriptions::kOmniboxUIHideSteadyStateUrlSchemeName,
flag_descriptions::kOmniboxUIHideSteadyStateUrlSchemeDescription, kOsAll,
FEATURE_VALUE_TYPE(omnibox::kHideSteadyStateUrlScheme)},
{"omnibox-ui-hide-steady-state-url-trivial-subdomains",
flag_descriptions::kOmniboxUIHideSteadyStateUrlTrivialSubdomainsName,
flag_descriptions::
kOmniboxUIHideSteadyStateUrlTrivialSubdomainsDescription,
kOsAll, FEATURE_VALUE_TYPE(omnibox::kHideSteadyStateUrlTrivialSubdomains)},
{"omnibox-ui-reveal-steady-state-url-path-query-and-ref-on-hover",
flag_descriptions::
kOmniboxUIRevealSteadyStateUrlPathQueryAndRefOnHoverName,
......
......@@ -3505,6 +3505,16 @@
"owners": [ "estark", "carlosil", "chrome-security-enamel@google.com" ],
"expiry_milestone": 89
},
{
"name": "omnibox-ui-hide-steady-state-url-scheme",
"owners": [ "tommycli", "chrome-omnibox-team@google.com" ],
"expiry_milestone": 76
},
{
"name": "omnibox-ui-hide-steady-state-url-trivial-subdomains",
"owners": [ "tommycli", "chrome-omnibox-team@google.com" ],
"expiry_milestone": 76
},
{
"name": "omnibox-ui-max-autocomplete-matches",
"owners": [ "jdonnelly", "chrome-omnibox-team@google.com" ],
......
......@@ -237,6 +237,8 @@ const base::Feature* kFeaturesExposedToJava[] = {
&omnibox::kAdaptiveSuggestionsCount,
&omnibox::kCompactSuggestions,
&omnibox::kDeferredKeyboardPopup,
&omnibox::kHideSteadyStateUrlScheme,
&omnibox::kHideSteadyStateUrlTrivialSubdomains,
&omnibox::kOmniboxAssistantVoiceSearch,
&omnibox::kOmniboxSearchEngineLogo,
&omnibox::kOmniboxSearchReadyIncognito,
......
......@@ -337,6 +337,10 @@ public abstract class ChromeFeatureList {
public static final String OMNIBOX_DEFERRED_KEYBOARD_POPUP = "OmniboxDeferredKeyboardPopup";
public static final String OMNIBOX_ENABLE_CLIPBOARD_PROVIDER_IMAGE_SUGGESTIONS =
"OmniboxEnableClipboardProviderImageSuggestions";
public static final String OMNIBOX_HIDE_SCHEME_IN_STEADY_STATE =
"OmniboxUIExperimentHideSteadyStateUrlScheme";
public static final String OMNIBOX_HIDE_TRIVIAL_SUBDOMAINS_IN_STEADY_STATE =
"OmniboxUIExperimentHideSteadyStateUrlTrivialSubdomains";
public static final String OMNIBOX_SEARCH_ENGINE_LOGO = "OmniboxSearchEngineLogo";
public static final String OMNIBOX_SEARCH_READY_INCOGNITO = "OmniboxSearchReadyIncognito";
public static final String OMNIBOX_SPARE_RENDERER = "OmniboxSpareRenderer";
......
......@@ -219,6 +219,11 @@ void LocationBarModelTest::NavigateAndCheckElided(const GURL& url) {
// Test URL display.
TEST_F(LocationBarModelTest, ShouldDisplayURL) {
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatures({omnibox::kHideSteadyStateUrlScheme,
omnibox::kHideSteadyStateUrlTrivialSubdomains},
{});
AddTab(browser(), GURL(url::kAboutBlankURL));
for (const TestItem& test_item : TestItems()) {
......@@ -229,6 +234,56 @@ TEST_F(LocationBarModelTest, ShouldDisplayURL) {
}
}
// Tests every combination of Steady State Elision flags.
TEST_F(LocationBarModelTest, SteadyStateElisionsFlags) {
AddTab(browser(), GURL(url::kAboutBlankURL));
// Hide Scheme and Hide Trivial Subdomains both Disabled.
{
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatures(
{}, {omnibox::kHideSteadyStateUrlScheme,
omnibox::kHideSteadyStateUrlTrivialSubdomains});
NavigateAndCheckText(GURL("https://www.google.com/"),
base::ASCIIToUTF16("https://www.google.com"),
base::ASCIIToUTF16("https://www.google.com"));
}
// Only Hide Scheme Enabled.
{
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatures(
{omnibox::kHideSteadyStateUrlScheme},
{omnibox::kHideSteadyStateUrlTrivialSubdomains});
NavigateAndCheckText(GURL("https://www.google.com/"),
base::ASCIIToUTF16("https://www.google.com"),
base::ASCIIToUTF16("www.google.com"));
}
// Only Hide Trivial Subdomains Enabled.
{
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatures(
{omnibox::kHideSteadyStateUrlTrivialSubdomains},
{omnibox::kHideSteadyStateUrlScheme});
NavigateAndCheckText(GURL("https://www.google.com/"),
base::ASCIIToUTF16("https://www.google.com"),
base::ASCIIToUTF16("https://google.com"));
}
// Hide Scheme and Hide Trivial Subdomains both Enabled.
{
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatures(
{omnibox::kHideSteadyStateUrlScheme,
omnibox::kHideSteadyStateUrlTrivialSubdomains},
{});
NavigateAndCheckText(GURL("https://www.google.com/"),
base::ASCIIToUTF16("https://www.google.com"),
base::ASCIIToUTF16("google.com"));
}
}
TEST_F(LocationBarModelTest, ShouldElideLongURLs) {
AddTab(browser(), GURL(url::kAboutBlankURL));
const std::string long_text(content::kMaxURLDisplayChars + 1024, '0');
......
......@@ -1729,8 +1729,8 @@ void OmniboxViewViews::DidFinishNavigation(
// Handling same-document or non-main-frame navigations is a bit tricky
// because they shouldn't change the current elision/unelision state:
// - If the user hadn't interacted with the previous page, then we don't
// need to do anything: the URL is already unelided and we're waiting for a
// user interaction to animate it to the simplified domain.
// need to do anything: the URL is currently unelided and we're waiting for
// a user interaction to animate it to the simplified domain.
// - If the user interacted with the page, and we are currently animating to
// the simplified domain as a result, we want to let the animation run
// undisturbed, eventually ending up in the elided state.
......@@ -1770,9 +1770,8 @@ void OmniboxViewViews::DidGetUserInteraction(
// designed to draw the user's attention and suggest that they can return to
// the omnibox to uncover the full URL.
// If the animation already exists, we've already processed a user interaction
// on this navigation and elided to the simplified domain (or are in the
// process of doing so). There's no need to do anything in this case.
// Only create and run the animation if we haven't already done so on an
// earlier call to this method.
if (IsURLEligibleForSimplifiedDomainEliding() &&
!elide_after_interaction_animation_) {
GetRenderText()->SetElideBehavior(gfx::NO_ELIDE);
......@@ -1783,7 +1782,8 @@ void OmniboxViewViews::DidGetUserInteraction(
0 /* delay_ms */);
}
// Now that the URL is being elided, create the animation to bring it back on
// hover (if enabled via field trial).
// hover (if enabled via field trial), if it hasn't already been created on an
// earlier call to this method.
if (OmniboxFieldTrial::ShouldRevealPathQueryRefOnHover() &&
!hover_elide_or_unelide_animation_) {
hover_elide_or_unelide_animation_ =
......
......@@ -62,8 +62,11 @@ base::string16 LocationBarModelImpl::GetURLForDisplay() const {
format_types |= url_formatter::kFormatUrlTrimAfterHost;
#endif
format_types |= url_formatter::kFormatUrlOmitHTTPS;
format_types |= url_formatter::kFormatUrlOmitTrivialSubdomains;
if (OmniboxFieldTrial::IsHideSteadyStateUrlSchemeEnabled())
format_types |= url_formatter::kFormatUrlOmitHTTPS;
if (OmniboxFieldTrial::IsHideSteadyStateUrlTrivialSubdomainsEnabled())
format_types |= url_formatter::kFormatUrlOmitTrivialSubdomains;
if (base::FeatureList::IsEnabled(omnibox::kHideFileUrlScheme))
format_types |= url_formatter::kFormatUrlOmitFileScheme;
......
......@@ -106,6 +106,11 @@ class LocationBarModelImplTest : public testing::Test {
TEST_F(LocationBarModelImplTest,
DisplayUrlAppliesFormattedStringWithEquivalentMeaning) {
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatures({omnibox::kHideSteadyStateUrlScheme,
omnibox::kHideSteadyStateUrlTrivialSubdomains},
{});
delegate()->SetURL(GURL("http://www.google.com/"));
// Verify that both the full formatted URL and the display URL add the test
......@@ -179,6 +184,11 @@ TEST_F(LocationBarModelImplTest, FormatsReaderModeUrls) {
#define MAYBE_PreventElisionWorks PreventElisionWorks
#endif
TEST_F(LocationBarModelImplTest, MAYBE_PreventElisionWorks) {
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatures({omnibox::kHideSteadyStateUrlScheme,
omnibox::kHideSteadyStateUrlTrivialSubdomains},
{});
delegate()->SetShouldPreventElision(true);
delegate()->SetURL(GURL("https://www.google.com/search?q=foo+query+unelide"));
......
......@@ -707,6 +707,15 @@ bool OmniboxFieldTrial::IsPedalSuggestionsEnabled() {
return base::FeatureList::IsEnabled(omnibox::kOmniboxPedalSuggestions);
}
bool OmniboxFieldTrial::IsHideSteadyStateUrlSchemeEnabled() {
return base::FeatureList::IsEnabled(omnibox::kHideSteadyStateUrlScheme);
}
bool OmniboxFieldTrial::IsHideSteadyStateUrlTrivialSubdomainsEnabled() {
return base::FeatureList::IsEnabled(
omnibox::kHideSteadyStateUrlTrivialSubdomains);
}
bool OmniboxFieldTrial::IsExperimentalKeywordModeEnabled() {
return base::FeatureList::IsEnabled(omnibox::kExperimentalKeywordMode);
}
......
......@@ -400,6 +400,13 @@ bool IsLooseMaxLimitOnDedicatedRowsEnabled();
// Returns true if the #omnibox-pedal-suggestions feature is enabled.
bool IsPedalSuggestionsEnabled();
// Returns true if either the steady-state elision flag for scheme is enabled.
bool IsHideSteadyStateUrlSchemeEnabled();
// Returns true if either the steady-state elision flag for trivial
// subdomains is enabled.
bool IsHideSteadyStateUrlTrivialSubdomainsEnabled();
// Simply a convenient wrapper for testing a flag. Used downstream for an
// assortment of keyword mode experiments.
bool IsExperimentalKeywordModeEnabled();
......
......@@ -30,6 +30,18 @@ const base::Feature kHideFileUrlScheme{
// need to show the file scheme.
enabled_by_default_desktop_only};
// Feature used to hide the scheme from steady state URLs displayed in the
// toolbar. It is restored during editing.
const base::Feature kHideSteadyStateUrlScheme{
"OmniboxUIExperimentHideSteadyStateUrlScheme",
base::FEATURE_ENABLED_BY_DEFAULT};
// Feature used to hide trivial subdomains from steady state URLs displayed in
// the toolbar. It is restored during editing.
const base::Feature kHideSteadyStateUrlTrivialSubdomains{
"OmniboxUIExperimentHideSteadyStateUrlTrivialSubdomains",
base::FEATURE_ENABLED_BY_DEFAULT};
// Feature used to enable local entity suggestions. Similar to rich entities but
// but location specific. E.g., typing 'starbucks near' could display the local
// entity suggestion 'starbucks near disneyland \n starbucks * Anaheim, CA'.
......
......@@ -13,6 +13,8 @@ namespace omnibox {
// Instead, use the categorized and alphabetized lists below this "big blob".
// You can create a new category if none of the existing ones fit.
extern const base::Feature kHideFileUrlScheme;
extern const base::Feature kHideSteadyStateUrlScheme;
extern const base::Feature kHideSteadyStateUrlTrivialSubdomains;
extern const base::Feature kOmniboxLocalEntitySuggestions;
extern const base::Feature kOmniboxReverseAnswers;
extern const base::Feature kOmniboxShortBookmarkSuggestions;
......
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