Do not create global GURL instances.

GURL constructor uses scheme registry which now DCHECKs if accessed
before being initialized with schemes.

Converted BraveStatsUpdater::g_base_update_url_ to std::string.
Converted SuggestedSitesProvider::suggested_sites_ to method local
static.
Converted oauth_url in binance_service.cc to const char[].

Also, moved inside functions in several unit tests.

Chromium change:

https://chromium.googlesource.com/chromium/src/+/2add7d44778f7be638639aeb26a3b14f0b222e09

commit 2add7d44778f7be638639aeb26a3b14f0b222e09
Author: Michael Thiessen <mthiesse@chromium.org>
Date:   Wed Feb 5 13:49:38 2020 +0000

    Lock SchemeRegistry on first use.

    This change locks the SchemeRegistry on first use, which required
    refactoring url_util.cc to differentiate between SchemeRegistry use for
    adding Schemes, and SchemeRegistry use for using Schemes.

    Tests can now only modify schemes after initialization by calling
    url::UnlockForTests(), which creates a scoped object that resets the
    schemes, so tests can't mistakenly leave global scheme state modified.

    This change required changes to browser startup to decouple scheme
    registration from ContentMain, found here:
    https://chromium-review.googlesource.com/c/chromium/src/+/1945926

    Doc: https://docs.google.com/document/d/1kDKqBaq-b6EbUm0F4ea7ARoksUcj1KUx3qxFuSXEwM4/edit

    Bug: 783819
This commit is contained in:
mkarolin
2020-05-26 09:52:30 -04:00
parent cfd7572961
commit 357902c33e
13 changed files with 70 additions and 50 deletions
+4 -3
View File
@@ -65,7 +65,7 @@ GURL GetUpdateURL(const GURL& base_update_url,
namespace brave {
GURL BraveStatsUpdater::g_base_update_url_(
std::string BraveStatsUpdater::g_base_update_url_(
"https://laptop-updates.brave.com/1/usage/brave-core");
BraveStatsUpdater::BraveStatsUpdater(PrefService* pref_service)
@@ -189,7 +189,7 @@ void BraveStatsUpdater::SendServerPing() {
auto stats_updater_params =
std::make_unique<brave::BraveStatsUpdaterParams>(pref_service_);
resource_request->url =
GetUpdateURL(g_base_update_url_, *stats_updater_params);
GetUpdateURL(GURL(g_base_update_url_), *stats_updater_params);
resource_request->load_flags =
net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES |
net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE |
@@ -206,7 +206,8 @@ void BraveStatsUpdater::SendServerPing() {
}
// static
void BraveStatsUpdater::SetBaseUpdateURLForTest(const GURL& base_update_url) {
void BraveStatsUpdater::SetBaseUpdateURLForTest(
const std::string& base_update_url) {
g_base_update_url_ = base_update_url;
}
+2 -2
View File
@@ -65,8 +65,8 @@ class BraveStatsUpdater {
void SendServerPing();
friend class ::BraveStatsUpdaterBrowserTest;
static void SetBaseUpdateURLForTest(const GURL& base_update_url);
static GURL g_base_update_url_;
static void SetBaseUpdateURLForTest(const std::string& base_update_url);
static std::string g_base_update_url_;
PrefService* pref_service_;
StatsUpdatedCallback stats_updated_callback_;
+1 -1
View File
@@ -63,7 +63,7 @@ class BraveStatsUpdaterBrowserTest : public InProcessBrowserTest {
void SetBaseUpdateURLForTest() {
std::unique_ptr<base::Environment> env(base::Environment::Create());
brave::BraveStatsUpdater::SetBaseUpdateURLForTest(
embedded_test_server()->GetURL("/1/usage/brave-core"));
embedded_test_server()->GetURL("/1/usage/brave-core").spec());
env->SetVar("BRAVE_REFERRALS_SERVER",
embedded_test_server()->host_port_pair().ToString());
env->SetVar("BRAVE_REFERRALS_LOCAL", "1"); // use http for local testing
@@ -20,13 +20,6 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
namespace {
const GURL kBraveURL("https://www.brave.com");
const GURL kBatURL("https://basicattentiontoken.org");
const GURL kGoogleURL("https://www.google.com");
const GURL kAbcURL("https://www.abc.com");
} // namespace
class BraveBrowsingDataRemoverDelegateTest : public testing::Test {
public:
void SetUp() override {
@@ -62,6 +55,10 @@ class BraveBrowsingDataRemoverDelegateTest : public testing::Test {
};
TEST_F(BraveBrowsingDataRemoverDelegateTest, ShieldsSettingsClearTest) {
const GURL kBraveURL("https://www.brave.com");
const GURL kBatURL("https://basicattentiontoken.org");
const GURL kGoogleURL("https://www.google.com");
const GURL kAbcURL("https://www.abc.com");
// Four settings are added.
// First two settings are shields settings in PLUGINS type.
// Javascript is not counted as shields type because it's stored to
@@ -30,13 +30,6 @@
#include "content/public/browser/host_zoom_map.h"
#endif
namespace {
const GURL kBraveURL("https://www.brave.com");
const GURL kBatURL("https://basicattentiontoken.org");
const GURL kGoogleURL("https://www.google.com");
const GURL kAbcURL("https://www.abc.com");
} // namespace
class BraveSiteSettingsCounterTest : public testing::Test {
public:
void SetUp() override {
@@ -92,6 +85,10 @@ class BraveSiteSettingsCounterTest : public testing::Test {
// Tests that the counter correctly counts each setting.
TEST_F(BraveSiteSettingsCounterTest, Count) {
const GURL kBraveURL("https://www.brave.com");
const GURL kBatURL("https://basicattentiontoken.org");
const GURL kGoogleURL("https://www.google.com");
const GURL kAbcURL("https://www.abc.com");
// Check below four settings for different host are counted properly.
map()->SetContentSettingDefaultScope(
kBraveURL, GURL(), ContentSettingsType::PLUGINS,
@@ -198,7 +198,11 @@ IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest, AllowScriptsOnceIframe) {
constexpr char kJavascriptSetParams[] =
"[\"block\", \"https://www.brave.com/\"]";
constexpr char kJavascriptGetParams[] = "[\"https://www.brave.com/\"]";
const GURL kBraveURL("https://www.brave.com");
const GURL& GetBraveURL() {
static const GURL kBraveURL("https://www.brave.com");
return kBraveURL;
}
// Test javascript content setting works properly via braveShields api.
IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest,
@@ -225,7 +229,7 @@ IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest,
// Check Block is set.
ContentSetting setting =
HostContentSettingsMapFactory::GetForProfile(browser()->profile())
->GetContentSetting(kBraveURL, GURL(),
->GetContentSetting(GetBraveURL(), GURL(),
ContentSettingsType::JAVASCRIPT, "");
EXPECT_EQ(setting, CONTENT_SETTING_BLOCK);
}
@@ -235,12 +239,13 @@ IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest,
PRE_ShieldSettingsPersistTest) {
HostContentSettingsMapFactory::GetForProfile(browser()->profile())
->SetContentSettingDefaultScope(
kBraveURL, GURL(), ContentSettingsType::PLUGINS,
GetBraveURL(), GURL(), ContentSettingsType::PLUGINS,
brave_shields::kHTTPUpgradableResources, CONTENT_SETTING_ALLOW);
ContentSetting setting =
HostContentSettingsMapFactory::GetForProfile(browser()->profile())
->GetContentSetting(kBraveURL, GURL(), ContentSettingsType::PLUGINS,
->GetContentSetting(GetBraveURL(), GURL(),
ContentSettingsType::PLUGINS,
brave_shields::kHTTPUpgradableResources);
EXPECT_EQ(setting, CONTENT_SETTING_ALLOW);
}
@@ -248,7 +253,8 @@ IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest,
IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest, ShieldSettingsPersistTest) {
ContentSetting setting =
HostContentSettingsMapFactory::GetForProfile(browser()->profile())
->GetContentSetting(kBraveURL, GURL(), ContentSettingsType::PLUGINS,
->GetContentSetting(GetBraveURL(), GURL(),
ContentSettingsType::PLUGINS,
brave_shields::kHTTPUpgradableResources);
EXPECT_EQ(setting, CONTENT_SETTING_ALLOW);
}
@@ -256,13 +262,13 @@ IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest, ShieldSettingsPersistTest) {
// Checks flash configuration isn't persisted across the sessions.
IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest, PRE_FlashPersistTest) {
HostContentSettingsMapFactory::GetForProfile(browser()->profile())
->SetContentSettingDefaultScope(kBraveURL, GURL(),
->SetContentSettingDefaultScope(GetBraveURL(), GURL(),
ContentSettingsType::PLUGINS,
std::string(), CONTENT_SETTING_ALLOW);
ContentSetting setting =
HostContentSettingsMapFactory::GetForProfile(browser()->profile())
->GetContentSetting(kBraveURL, GURL(),
->GetContentSetting(GetBraveURL(), GURL(),
ContentSettingsType::PLUGINS, std::string());
EXPECT_EQ(setting, CONTENT_SETTING_ALLOW);
}
@@ -270,7 +276,7 @@ IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest, PRE_FlashPersistTest) {
IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest, FlashPersistTest) {
ContentSetting setting =
HostContentSettingsMapFactory::GetForProfile(browser()->profile())
->GetContentSetting(kBraveURL, GURL(),
->GetContentSetting(GetBraveURL(), GURL(),
ContentSettingsType::PLUGINS, std::string());
EXPECT_EQ(setting, CONTENT_SETTING_BLOCK);
}
@@ -53,8 +53,17 @@ class MockBrowserClient : public content::ContentBrowserClient {
}
};
GURL magnet_url("magnet:?xt=urn:btih:dd8255ecdc7ca55fb0bbf81323d87062db1f6d1c&dn=Big+Buck+Bunny&tr=udp%3A%2F%2Fexplodie.org%3A6969&tr=udp%3A%2F%2Ftracker.coppersurfer.tk%3A6969&tr=udp%3A%2F%2Ftracker.empire-js.us%3A1337&tr=udp%3A%2F%2Ftracker.leechers-paradise.org%3A6969&tr=udp%3A%2F%2Ftracker.opentrackr.org%3A1337&tr=wss%3A%2F%2Ftracker.btorrent.xyz&tr=wss%3A%2F%2Ftracker.fastcast.nz&tr=wss%3A%2F%2Ftracker.openwebtorrent.com&ws=https%3A%2F%2Fwebtorrent.io%2Ftorrents%2F&xs=https%3A%2F%2Fwebtorrent.io%2Ftorrents%2Fbig-buck-bunny.torrent"); // NOLINT
GURL torrent_url("https://webtorrent.io/torrents/big-buck-bunny.torrent");
const GURL& GetMagnetUrl() {
static const GURL magnet_url(
"magnet:?xt=urn:btih:dd8255ecdc7ca55fb0bbf81323d87062db1f6d1c&dn=Big+Buck+Bunny&tr=udp%3A%2F%2Fexplodie.org%3A6969&tr=udp%3A%2F%2Ftracker.coppersurfer.tk%3A6969&tr=udp%3A%2F%2Ftracker.empire-js.us%3A1337&tr=udp%3A%2F%2Ftracker.leechers-paradise.org%3A6969&tr=udp%3A%2F%2Ftracker.opentrackr.org%3A1337&tr=wss%3A%2F%2Ftracker.btorrent.xyz&tr=wss%3A%2F%2Ftracker.fastcast.nz&tr=wss%3A%2F%2Ftracker.openwebtorrent.com&ws=https%3A%2F%2Fwebtorrent.io%2Ftorrents%2F&xs=https%3A%2F%2Fwebtorrent.io%2Ftorrents%2Fbig-buck-bunny.torrent"); // NOLINT
return magnet_url;
}
const GURL& GetTorrentUrl() {
static const GURL torrent_url(
"https://webtorrent.io/torrents/big-buck-bunny.torrent");
return torrent_url;
}
} // namespace
@@ -141,12 +150,12 @@ TEST_F(BraveWebTorrentNavigationThrottleUnitTest,
web_contents_tester()->NavigateAndCommit(GURL("http://example.com"));
content::RenderFrameHost* host =
render_frame_host_tester(main_rfh())->AppendChild("child");
content::MockNavigationHandle test_handle(torrent_url, host);
content::MockNavigationHandle test_handle(GetTorrentUrl(), host);
test_handle.set_starting_site_instance(host->GetSiteInstance());
auto throttle =
std::make_unique<BraveWebTorrentNavigationThrottle>(&test_handle);
EXPECT_EQ(NavigationThrottle::DEFER, throttle->WillStartRequest().action())
<< torrent_url;
<< GetTorrentUrl();
}
// Tests the case of loading a torrent without having the extension
@@ -156,12 +165,12 @@ TEST_F(BraveWebTorrentNavigationThrottleUnitTest,
web_contents_tester()->NavigateAndCommit(GURL("http://example.com"));
content::RenderFrameHost* host =
render_frame_host_tester(main_rfh())->AppendChild("child");
content::MockNavigationHandle test_handle(magnet_url, host);
content::MockNavigationHandle test_handle(GetMagnetUrl(), host);
test_handle.set_starting_site_instance(host->GetSiteInstance());
auto throttle =
std::make_unique<BraveWebTorrentNavigationThrottle>(&test_handle);
EXPECT_EQ(NavigationThrottle::DEFER, throttle->WillStartRequest().action())
<< magnet_url;
<< GetMagnetUrl();
}
@@ -172,12 +181,12 @@ TEST_F(BraveWebTorrentNavigationThrottleUnitTest, WebTorrentUrlInstalled) {
web_contents_tester()->NavigateAndCommit(GURL("http://example.com"));
content::RenderFrameHost* host =
render_frame_host_tester(main_rfh())->AppendChild("child");
content::MockNavigationHandle test_handle(magnet_url, host);
content::MockNavigationHandle test_handle(GetMagnetUrl(), host);
test_handle.set_starting_site_instance(host->GetSiteInstance());
auto throttle =
std::make_unique<BraveWebTorrentNavigationThrottle>(&test_handle);
EXPECT_EQ(NavigationThrottle::PROCEED, throttle->WillStartRequest().action())
<< magnet_url;
<< GetMagnetUrl();
}
// Tests the case of loading a torrent when the WebTorrent is explicitly
@@ -187,12 +196,12 @@ TEST_F(BraveWebTorrentNavigationThrottleUnitTest, WebTorrentDisabledByPref) {
web_contents_tester()->NavigateAndCommit(GURL("http://example.com"));
content::RenderFrameHost* host =
render_frame_host_tester(main_rfh())->AppendChild("child");
content::MockNavigationHandle test_handle(magnet_url, host);
content::MockNavigationHandle test_handle(GetMagnetUrl(), host);
test_handle.set_starting_site_instance(host->GetSiteInstance());
auto throttle =
std::make_unique<BraveWebTorrentNavigationThrottle>(&test_handle);
EXPECT_EQ(NavigationThrottle::PROCEED,
throttle->WillStartRequest().action()) << magnet_url;
throttle->WillStartRequest().action()) << GetMagnetUrl();
}
} // namespace extensions
@@ -11,7 +11,10 @@
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/content_settings/core/common/content_settings_pattern.h"
const GURL kBraveURL("https://www.brave.com");
const GURL& GetBraveURL() {
static const GURL kBraveURL("https://www.brave.com");
return kBraveURL;
}
class BraveContentSettingsRegistryBrowserTest : public InProcessBrowserTest {
public:
@@ -34,13 +37,13 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsRegistryBrowserTest,
WithoutWildcardContentSetting) {
ContentSetting brave_url_shields_setting =
content_settings()->GetContentSetting(
kBraveURL, kBraveURL, ContentSettingsType::PLUGINS,
GetBraveURL(), GetBraveURL(), ContentSettingsType::PLUGINS,
brave_shields::kBraveShields);
EXPECT_EQ(CONTENT_SETTING_DEFAULT, brave_url_shields_setting);
ContentSetting brave_url_shields_setting_private =
private_content_settings()->GetContentSetting(
kBraveURL, kBraveURL, ContentSettingsType::PLUGINS,
GetBraveURL(), GetBraveURL(), ContentSettingsType::PLUGINS,
brave_shields::kBraveShields);
EXPECT_EQ(CONTENT_SETTING_DEFAULT, brave_url_shields_setting_private);
}
@@ -48,7 +51,7 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsRegistryBrowserTest,
IN_PROC_BROWSER_TEST_F(BraveContentSettingsRegistryBrowserTest,
WithBraveShieldsContentSetting) {
ContentSettingsPattern brave_url_pattern =
ContentSettingsPattern::FromURL(kBraveURL);
ContentSettingsPattern::FromURL(GetBraveURL());
content_settings()->SetContentSettingCustomScope(
brave_url_pattern,
@@ -59,13 +62,13 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsRegistryBrowserTest,
ContentSetting brave_url_shields_setting =
content_settings()->GetContentSetting(
kBraveURL, kBraveURL, ContentSettingsType::PLUGINS,
GetBraveURL(), GetBraveURL(), ContentSettingsType::PLUGINS,
brave_shields::kBraveShields);
EXPECT_EQ(CONTENT_SETTING_ALLOW, brave_url_shields_setting);
ContentSetting brave_url_shields_setting_private =
private_content_settings()->GetContentSetting(
kBraveURL, kBraveURL, ContentSettingsType::PLUGINS,
GetBraveURL(), GetBraveURL(), ContentSettingsType::PLUGINS,
brave_shields::kBraveShields);
EXPECT_EQ(CONTENT_SETTING_ALLOW, brave_url_shields_setting_private);
}
+2
View File
@@ -9,8 +9,10 @@
#include "content/public/common/url_constants.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
#include "url/url_util.h"
TEST(BraveContentClientTest, AdditionalSchemesTest) {
url::ScopedSchemeRegistryForTests scoped_registry;
BraveContentClient content_client;
content::SetContentClient(&content_client);
content::ReRegisterContentSchemesForTests();
@@ -42,7 +42,7 @@ const char gateway_host[] = "www.binance.com";
const char oauth_callback[] = "com.brave.binance://authorization";
const char oauth_scope[] =
"user:email,user:address,asset:balance,asset:ocbs";
const GURL oauth_url("https://accounts.binance.com/en/oauth/authorize");
const char oauth_url[] = "https://accounts.binance.com/en/oauth/authorize";
const unsigned int kRetriesCountOnNetworkChange = 1;
net::NetworkTrafficAnnotationTag GetNetworkTrafficAnnotationTag() {
@@ -69,7 +69,8 @@ void SuggestedSitesProvider::Start(const AutocompleteInput& input,
}
};
std::for_each(suggested_sites_.begin(), suggested_sites_.end(),
const auto& suggested_sites = GetSuggestedSites();
std::for_each(suggested_sites.begin(), suggested_sites.end(),
check_add_match);
}
@@ -28,10 +28,10 @@ class SuggestedSitesProvider : public AutocompleteProvider {
private:
~SuggestedSitesProvider() override;
static std::vector<SuggestedSitesMatch> suggested_sites_;
static const int kRelevance;
const std::vector<SuggestedSitesMatch>& GetSuggestedSites();
void AddMatch(const SuggestedSitesMatch& match,
const ACMatchClassifications& styles);
@@ -7,8 +7,9 @@
#include "base/strings/utf_string_conversions.h"
std::vector<SuggestedSitesMatch>
SuggestedSitesProvider::suggested_sites_ = {
const std::vector<SuggestedSitesMatch>&
SuggestedSitesProvider::GetSuggestedSites() {
static const std::vector<SuggestedSitesMatch> suggested_sites = {
{
SuggestedSitesMatch(
"binance.com",
@@ -143,4 +144,7 @@ SuggestedSitesProvider::suggested_sites_ = {
"?offer_id=24&aff_id=3494"),
true)
},
};
};
return suggested_sites;
}