Skip to content

Commit

Permalink
Merge pull request #2471 from brave/disable_field_trials
Browse files Browse the repository at this point in the history
Issue 4552: Restore features to correct state after disabling field trials
  • Loading branch information
jumde authored May 29, 2019
2 parents 4b7fd4b + 58a37c5 commit cda823c
Show file tree
Hide file tree
Showing 8 changed files with 194 additions and 11 deletions.
6 changes: 6 additions & 0 deletions app/brave_command_line_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,9 @@ void BraveCommandLineHelper::AppendCSV(
}
command_line_.AppendSwitchASCII(switch_key, ss.str());
}

void BraveCommandLineHelper::AppendSwitchASCII(const char* switch_key,
const char* value) {
if (!command_line_.HasSwitch(switch_key))
command_line_.AppendSwitchASCII(switch_key, value);
}
1 change: 1 addition & 0 deletions app/brave_command_line_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class BraveCommandLineHelper {
inline ~BraveCommandLineHelper() = default;

void AppendSwitch(const char* switch_key);
void AppendSwitchASCII(const char* switch_key, const char* value);
void AppendFeatures(const std::unordered_set<const char*>& enabled,
const std::unordered_set<const char*>& disabled);

Expand Down
9 changes: 4 additions & 5 deletions app/brave_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@
#include "chrome/common/chrome_switches.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/autofill/core/common/autofill_payments_features.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/password_manager/core/common/password_manager_features.h"
#include "components/unified_consent/feature.h"
#include "content/public/common/content_features.h"
#include "extensions/common/extension_features.h"
#include "gpu/config/gpu_finch_features.h"
#include "services/network/public/cpp/features.h"
#include "third_party/widevine/cdm/buildflags.h"
#include "ui/base/ui_base_features.h"
Expand Down Expand Up @@ -132,24 +131,24 @@ bool BraveMainDelegate::BasicStartupComplete(int* exit_code) {
command_line.AppendSwitch(switches::kDisableDomainReliability);
command_line.AppendSwitch(switches::kDisableChromeGoogleURLTrackingClient);
command_line.AppendSwitch(switches::kNoPings);
command_line.AppendSwitchASCII(switches::kExtensionsInstallVerification,
switches::kExtensionContentVerificationEnforceStrict);

// Enabled features.
const std::unordered_set<const char*> enabled_features = {
#if BUILDFLAG(ENABLE_EXTENSIONS)
extensions_features::kNewExtensionUpdaterService.name,
#endif
features::kDesktopPWAWindowing.name,
omnibox::kSimplifyHttpsIndicator.name,
};

// Disabled features.
const std::unordered_set<const char*> disabled_features = {
autofill::features::kAutofillSaveCardSignInAfterLocalSave.name,
autofill::features::kAutofillServerCommunication.name,
features::kAudioServiceOutOfProcess.name,
features::kDefaultEnableOopRasterization.name,
network::features::kNetworkService.name,
unified_consent::kUnifiedConsent.name,
features::kLookalikeUrlNavigationSuggestionsUI.name,
};

command_line.AppendFeatures(enabled_features, disabled_features);
Expand Down
25 changes: 20 additions & 5 deletions app/brave_main_delegate_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "chrome/browser/domain_reliability/service_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/browser/domain_reliability/service_factory.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/autofill/core/common/autofill_payments_features.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/unified_consent/feature.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/common/content_features.h"
#include "content/public/common/web_preferences.h"
#include "extensions/common/extension_features.h"
#include "gpu/config/gpu_finch_features.h"
#include "services/network/public/cpp/features.h"

Expand All @@ -25,12 +27,12 @@ IN_PROC_BROWSER_TEST_F(BraveMainDelegateBrowserTest,
EXPECT_TRUE(base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableDomainReliability));
EXPECT_FALSE(domain_reliability::DomainReliabilityServiceFactory::
ShouldCreateService());
ShouldCreateService());
}

IN_PROC_BROWSER_TEST_F(BraveMainDelegateBrowserTest, DisableHyperlinkAuditing) {
EXPECT_TRUE(base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kNoPings));
EXPECT_TRUE(
base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoPings));
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
const content::WebPreferences prefs =
Expand All @@ -52,3 +54,16 @@ IN_PROC_BROWSER_TEST_F(BraveMainDelegateBrowserTest, DisabledFeatures) {
for (const auto* feature : disabled_features)
EXPECT_FALSE(base::FeatureList::IsEnabled(*feature));
}

IN_PROC_BROWSER_TEST_F(BraveMainDelegateBrowserTest, EnabledFeatures) {
const base::Feature* enabled_features[] = {
#if BUILDFLAG(ENABLE_EXTENSIONS)
&extensions_features::kNewExtensionUpdaterService,
#endif
&features::kDesktopPWAWindowing,
&omnibox::kSimplifyHttpsIndicator,
};

for (const auto* feature : enabled_features)
EXPECT_TRUE(base::FeatureList::IsEnabled(*feature));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/* Copyright (c) 2019 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "chrome/browser/ui/views/location_bar/location_bar_view.h"

#include "base/strings/string_util.h"
#include "chrome/browser/ssl/security_state_tab_helper.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/omnibox/browser/location_bar_model_impl.h"
#include "components/omnibox/common/omnibox_features.h"
#include "content/public/test/url_loader_interceptor.h"
#include "net/cert/ct_policy_status.h"
#include "net/ssl/ssl_info.h"
#include "net/test/cert_test_util.h"
#include "net/test/test_data_directory.h"

const char kMockSecureHostname[] = "example-secure.test";
const GURL kMockSecureURL = GURL("https://example-secure.test");

class SecurityIndicatorTest : public InProcessBrowserTest {
public:
SecurityIndicatorTest() : InProcessBrowserTest(), cert_(nullptr) {}

void SetUpInProcessBrowserTestFixture() override {
cert_ =
net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
ASSERT_TRUE(cert_);
ASSERT_TRUE(embedded_test_server()->Start());
}

LocationBarView* GetLocationBarView() {
BrowserView* browser_view =
BrowserView::GetBrowserViewForBrowser(browser());
return browser_view->GetLocationBarView();
}

void SetUpInterceptor(net::CertStatus cert_status) {
url_loader_interceptor_ = std::make_unique<content::URLLoaderInterceptor>(
base::BindRepeating(&SecurityIndicatorTest::InterceptURLLoad,
base::Unretained(this), cert_status));
}

void ResetInterceptor() { url_loader_interceptor_.reset(); }

bool InterceptURLLoad(net::CertStatus cert_status,
content::URLLoaderInterceptor::RequestParams* params) {
if (params->url_request.url.host() != kMockSecureHostname)
return false;
net::SSLInfo ssl_info;
ssl_info.cert = cert_;
ssl_info.cert_status = cert_status;
ssl_info.ct_policy_compliance =
net::ct::CTPolicyCompliance::CT_POLICY_COMPLIES_VIA_SCTS;
network::ResourceResponseHead resource_response;
resource_response.mime_type = "text/html";
resource_response.ssl_info = ssl_info;
params->client->OnReceiveResponse(resource_response);
// Send an empty response's body. This pipe is not filled with data.
mojo::DataPipe pipe;
params->client->OnStartLoadingResponseBody(std::move(pipe.consumer_handle));
network::URLLoaderCompletionStatus completion_status;
completion_status.ssl_info = ssl_info;
params->client->OnComplete(completion_status);
return true;
}

private:
scoped_refptr<net::X509Certificate> cert_;
std::unique_ptr<content::URLLoaderInterceptor> url_loader_interceptor_;

DISALLOW_COPY_AND_ASSIGN(SecurityIndicatorTest);
};

IN_PROC_BROWSER_TEST_F(SecurityIndicatorTest, CheckIndicatorText) {
const GURL kMockNonsecureURL =
embedded_test_server()->GetURL("example.test", "/");
const base::string16 kEmptyString = base::EmptyString16();

const struct {
GURL url;
net::CertStatus cert_status;
security_state::SecurityLevel security_level;
bool should_show_text;
base::string16 indicator_text;
} cases[]{// Default
{kMockSecureURL, net::CERT_STATUS_IS_EV, security_state::EV_SECURE,
false, kEmptyString},
{kMockSecureURL, 0, security_state::SECURE, false, kEmptyString},
{kMockNonsecureURL, 0, security_state::NONE, false, kEmptyString}};

content::WebContents* tab =
browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(tab);

// After SetUpInterceptor() is called, requests to this hostname will be
// mocked and use specified certificate validation results. This allows tests
// to mock Extended Validation (EV) certificate connections.
SecurityStateTabHelper* helper = SecurityStateTabHelper::FromWebContents(tab);
ASSERT_TRUE(helper);
LocationBarView* location_bar_view = GetLocationBarView();

for (const auto& c : cases) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(omnibox::kSimplifyHttpsIndicator);
SetUpInterceptor(c.cert_status);
ui_test_utils::NavigateToURL(browser(), c.url);
EXPECT_EQ(c.security_level, helper->GetSecurityLevel());
EXPECT_EQ(c.should_show_text,
location_bar_view->location_icon_view()->ShouldShowLabel());
EXPECT_EQ(c.indicator_text,
location_bar_view->location_icon_view()->GetText());
ResetInterceptor();
}
}
22 changes: 22 additions & 0 deletions chromium_src/components/omnibox/browser/location_bar_model_impl.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/* Copyright (c) 2019 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "components/omnibox/browser/location_bar_model_impl.h"

#include "components/omnibox/browser/omnibox_field_trial.h"

#define GetFieldTrialParamValueByFeature \
GetFieldTrialParamValueByFeature_ChromiumImpl

namespace base {
std::string GetFieldTrialParamValueByFeature(const base::Feature& feature,
const std::string& param_name) {
return OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterBothToLock;
}
} // namespace base

#include "../../../../../../../components/omnibox/browser/location_bar_model_impl.cc" // NOLINT

#undef GetFieldTrialParamValueByFeature
20 changes: 20 additions & 0 deletions chromium_src/components/variations/service/field_trial_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* Copyright (c) 2019 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "components/variations/service/buildflags.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace {
typedef testing::Test FieldTrialsTest;

TEST_F(FieldTrialsTest, FieldTrialsTestingEnabled) {
bool enabled = false;
#if BUILDFLAG(FIELDTRIAL_TESTING_ENABLED)
enabled = true
#endif // BUILDFLAG(FIELDTRIAL_TESTING_ENABLED)
EXPECT_FALSE(enabled);
}

} // namespace
4 changes: 3 additions & 1 deletion test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ test("brave_unit_tests") {
"//brave/chromium_src/components/metrics/enabled_state_provider_unittest.cc",
"//brave/chromium_src/components/search_engines/brave_template_url_prepopulate_data_unittest.cc",
"//brave/chromium_src/components/search_engines/brave_template_url_service_util_unittest.cc",
"//brave/chromium_src/components/translate/core/browser/translate_manager_unittest.cc",
"//brave/chromium_src/components/variations/service/field_trial_unittest.cc",
"//brave/chromium_src/components/version_info/brave_version_info_unittest.cc",
"//brave/chromium_src/components/translate/core/browser/translate_manager_unittest.cc",
"//brave/chromium_src/net/cookies/brave_canonical_cookie_unittest.cc",
"//brave/common/brave_content_client_unittest.cc",
"//brave/common/importer/brave_mock_importer_bridge.cc",
Expand Down Expand Up @@ -304,6 +305,7 @@ test("brave_browser_tests") {
"//brave/browser/autocomplete/brave_autocomplete_provider_client_browsertest.cc",
"//brave/browser/brave_scheme_load_browsertest.cc",
"//brave/chromium_src/chrome/browser/google/chrome_google_url_tracker_client_browsertest.cc",
"//brave/chromium_src/chrome/browser/ui/views/location_bar/location_bar_view_browsertest.cc",
"//brave/chromium_src/components/content_settings/core/browser/brave_content_settings_registry_browsertest.cc",
"//brave/chromium_src/third_party/blink/renderer/modules/battery/navigator_batterytest.cc",
"//brave/chromium_src/third_party/blink/renderer/modules/bluetooth/navigator_bluetoothtest.cc",
Expand Down

0 comments on commit cda823c

Please sign in to comment.