Commit f2af1ee6 authored by Ernesto García's avatar Ernesto García Committed by Commit Bot

omnibox: Exclude enterprise-enrolled devices from simplified domain display

For a 1% experiment with simplified domain display, we should exclude
enterprise enrolled devices to not disrupt enterprise environments when
collecting data.
This CL includes the following settings to achieve that purpose:
- Added a IsEnterpriseManaged() virtual method to OmniboxViewViews, in order to
filter whether a device is enterprise enrolled or not
- Added |is_enterprise_managed_for_testing| variable to toggle during
testing. It is by default FALSE, so, it should not affect previous
tests
- Added a SetEnterpriseManagedForTesting() method in TestOmniboxView to manually toggle the
|is_enterprise_managed_for_testing| flag, which is used in an overrode version of IsEnterpriseManaged()
- Add an special test to validate that URL is not unelided even when
field trials are enabled but the device is enterprise managed

Bug: 1109243
Change-Id: I79b47374de6c9eb780b5e56bef6408d3130c4168
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2348578
Commit-Queue: Ernesto García <ernestognw@google.com>
Reviewed-by: default avatarCarlos IL <carlosil@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797930}
parent a49cf47b
...@@ -99,6 +99,14 @@ ...@@ -99,6 +99,14 @@
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#endif #endif
#if defined(OS_WIN) || defined(OS_MAC)
#include "base/enterprise_util.h"
#elif defined(OS_CHROMEOS)
#include "chrome/browser/browser_process.h"
#include "chrome/browser/browser_process_platform_part_chromeos.h"
#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
#endif
using metrics::OmniboxEventProto; using metrics::OmniboxEventProto;
namespace { namespace {
...@@ -2473,6 +2481,8 @@ gfx::Range OmniboxViewViews::GetSimplifiedDomainBounds( ...@@ -2473,6 +2481,8 @@ gfx::Range OmniboxViewViews::GetSimplifiedDomainBounds(
} }
bool OmniboxViewViews::IsURLEligibleForSimplifiedDomainEliding() { bool OmniboxViewViews::IsURLEligibleForSimplifiedDomainEliding() {
if (IsEnterpriseManaged())
return false;
if (HasFocus() || model()->user_input_in_progress()) if (HasFocus() || model()->user_input_in_progress())
return false; return false;
if (!model()->CurrentTextIsURL()) if (!model()->CurrentTextIsURL())
...@@ -2491,6 +2501,18 @@ bool OmniboxViewViews::IsURLEligibleForSimplifiedDomainEliding() { ...@@ -2491,6 +2501,18 @@ bool OmniboxViewViews::IsURLEligibleForSimplifiedDomainEliding() {
host.is_nonempty(); host.is_nonempty();
} }
bool OmniboxViewViews::IsEnterpriseManaged() {
#if defined(OS_WIN) || defined(OS_MAC)
return base::IsMachineExternallyManaged();
#elif defined(OS_CHROMEOS)
policy::BrowserPolicyConnectorChromeOS* connector =
g_browser_process->platform_part()->browser_policy_connector_chromeos();
return connector && connector->IsEnterpriseManaged();
#endif
return false;
}
void OmniboxViewViews::ResetToHideOnInteraction() { void OmniboxViewViews::ResetToHideOnInteraction() {
if (!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction() || if (!OmniboxFieldTrial::ShouldHidePathQueryRefOnInteraction() ||
model()->ShouldPreventElision()) { model()->ShouldPreventElision()) {
......
...@@ -488,6 +488,11 @@ class OmniboxViewViews : public OmniboxView, ...@@ -488,6 +488,11 @@ class OmniboxViewViews : public OmniboxView,
// model()->ShouldPreventElision() if applicable. // model()->ShouldPreventElision() if applicable.
bool IsURLEligibleForSimplifiedDomainEliding(); bool IsURLEligibleForSimplifiedDomainEliding();
// Returns true if device is enrolled in enterprise management. This is
// used to exclude those devices from simplified domain experiments, to
// avoid enterprise environment disruption.
virtual bool IsEnterpriseManaged();
// When certain field trials are enabled, the URL is shown on page load // When certain field trials are enabled, the URL is shown on page load
// and elided to a simplified domain when the user interacts with the page. // and elided to a simplified domain when the user interacts with the page.
// This method resets back to the on-page-load state. That is, it unhides the // This method resets back to the on-page-load state. That is, it unhides the
......
...@@ -92,6 +92,10 @@ class TestingOmniboxView : public OmniboxViewViews { ...@@ -92,6 +92,10 @@ class TestingOmniboxView : public OmniboxViewViews {
// OmniboxViewViews: // OmniboxViewViews:
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {} void GetAccessibleNodeData(ui::AXNodeData* node_data) override {}
void OnThemeChanged() override; void OnThemeChanged() override;
// Forces enterprise enrolled status on or off using
// |is_enterprise_managed_for_testing|. If not called, enterprise
// enrolled status in tests defaults to off.
void SetEnterpriseManagedForTesting(bool enterprise_managed);
using OmniboxView::OnInlineAutocompleteTextMaybeChanged; using OmniboxView::OnInlineAutocompleteTextMaybeChanged;
...@@ -104,6 +108,7 @@ class TestingOmniboxView : public OmniboxViewViews { ...@@ -104,6 +108,7 @@ class TestingOmniboxView : public OmniboxViewViews {
void SetEmphasis(bool emphasize, const Range& range) override; void SetEmphasis(bool emphasize, const Range& range) override;
void UpdateSchemeStyle(const Range& range) override; void UpdateSchemeStyle(const Range& range) override;
void ApplyColor(SkColor color, const gfx::Range& range) override; void ApplyColor(SkColor color, const gfx::Range& range) override;
bool IsEnterpriseManaged() override;
size_t update_popup_call_count_ = 0; size_t update_popup_call_count_ = 0;
base::string16 update_popup_text_; base::string16 update_popup_text_;
...@@ -122,6 +127,10 @@ class TestingOmniboxView : public OmniboxViewViews { ...@@ -122,6 +127,10 @@ class TestingOmniboxView : public OmniboxViewViews {
// SetEmphasis() logs whether the base color of the text is emphasized. // SetEmphasis() logs whether the base color of the text is emphasized.
bool base_text_emphasis_; bool base_text_emphasis_;
// Set to false by default since most tests test the non-enterprise
// enrolled behavior.
bool is_enterprise_managed_for_testing = false;
DISALLOW_COPY_AND_ASSIGN(TestingOmniboxView); DISALLOW_COPY_AND_ASSIGN(TestingOmniboxView);
}; };
...@@ -201,6 +210,15 @@ void TestingOmniboxView::ApplyColor(SkColor color, const gfx::Range& range) { ...@@ -201,6 +210,15 @@ void TestingOmniboxView::ApplyColor(SkColor color, const gfx::Range& range) {
OmniboxViewViews::ApplyColor(color, range); OmniboxViewViews::ApplyColor(color, range);
} }
void TestingOmniboxView::SetEnterpriseManagedForTesting(
bool enterprise_managed) {
is_enterprise_managed_for_testing = enterprise_managed;
}
bool TestingOmniboxView::IsEnterpriseManaged() {
return is_enterprise_managed_for_testing;
}
// TestingOmniboxEditController ----------------------------------------------- // TestingOmniboxEditController -----------------------------------------------
class TestingOmniboxEditController : public ChromeOmniboxEditController { class TestingOmniboxEditController : public ChromeOmniboxEditController {
...@@ -324,6 +342,7 @@ class OmniboxViewViewsTest : public OmniboxViewViewsTestBase { ...@@ -324,6 +342,7 @@ class OmniboxViewViewsTest : public OmniboxViewViewsTestBase {
location_bar_model()->set_url_for_display(kSimplifiedDomainDisplayUrl); location_bar_model()->set_url_for_display(kSimplifiedDomainDisplayUrl);
omnibox_view()->model()->ResetDisplayTexts(); omnibox_view()->model()->ResetDisplayTexts();
omnibox_view()->RevertAll(); omnibox_view()->RevertAll();
omnibox_view()->SetEnterpriseManagedForTesting(false);
// Call OnThemeChanged() to create the animations. // Call OnThemeChanged() to create the animations.
omnibox_view()->OnThemeChanged(); omnibox_view()->OnThemeChanged();
} }
...@@ -1608,6 +1627,38 @@ TEST_F(OmniboxViewViewsNoSimplifiedDomainTest, UrlNotSimplifiedByDefault) { ...@@ -1608,6 +1627,38 @@ TEST_F(OmniboxViewViewsNoSimplifiedDomainTest, UrlNotSimplifiedByDefault) {
gfx::Range(0, kSimplifiedDomainDisplayUrl.size()))); gfx::Range(0, kSimplifiedDomainDisplayUrl.size())));
} }
class OmniboxViewViewsNoSimplifiedDomainOnEnterpriseManagedDevicesTest
: public OmniboxViewViewsTest {
public:
OmniboxViewViewsNoSimplifiedDomainOnEnterpriseManagedDevicesTest()
: OmniboxViewViewsTest(
std::vector<FeatureAndParams>(
{{omnibox::kRevealSteadyStateUrlPathQueryAndRefOnHover, {}},
{omnibox::kMaybeElideToRegistrableDomain,
// Ensure all domains are elidable by policy.
{{"max_unelided_host_length", "0"}}}}),
{}) {}
OmniboxViewViewsNoSimplifiedDomainOnEnterpriseManagedDevicesTest(
const OmniboxViewViewsNoSimplifiedDomainOnEnterpriseManagedDevicesTest&) =
delete;
OmniboxViewViewsNoSimplifiedDomainOnEnterpriseManagedDevicesTest& operator=(
const OmniboxViewViewsNoSimplifiedDomainOnEnterpriseManagedDevicesTest&) =
delete;
};
// Tests that when device is enterprise managed, URL components are not hidden
// even when field trials are enabled
TEST_F(OmniboxViewViewsNoSimplifiedDomainOnEnterpriseManagedDevicesTest,
UrlNotSimplifiedInEnterpriseManagedDevices) {
SetUpSimplifiedDomainTest();
omnibox_view()->SetEnterpriseManagedForTesting(true);
omnibox_view()->EmphasizeURLComponents();
ASSERT_NO_FATAL_FAILURE(ExpectUnelidedFromSimplifiedDomain(
omnibox_view()->GetRenderText(),
gfx::Range(0, kSimplifiedDomainDisplayUrl.size())));
}
class OmniboxViewViewsRevealOnHoverTest class OmniboxViewViewsRevealOnHoverTest
: public OmniboxViewViewsTest, : public OmniboxViewViewsTest,
public ::testing::WithParamInterface<std::pair<bool, bool>> { public ::testing::WithParamInterface<std::pair<bool, bool>> {
......
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