Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use brave:// for the virtual url and chrome:// for the internal u… #1385

Merged
merged 2 commits into from
Jan 31, 2019
Merged

use brave:// for the virtual url and chrome:// for the internal u… #1385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
742129b
use `brave://` for the virtual url and `chrome://` for the internal u…
bridiver Jan 18, 2019
0c92190
fix lint errors
bridiver Jan 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 66 additions & 29 deletions browser/brave_content_browser_client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
/* 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 "brave/browser/brave_content_browser_client.h"

#include <string>
#include <utility>

#include "base/bind.h"
#include "base/json/json_reader.h"
#include "base/task/post_task.h"
Expand Down Expand Up @@ -43,6 +47,7 @@
#include "ui/base/resource/resource_bundle.h"

using content::BrowserThread;
using content::ContentBrowserClient;
using content::RenderFrameHost;
using content::WebContents;
using brave_shields::BraveShieldsWebContentsObserver;
Expand All @@ -61,44 +66,67 @@ using extensions::ChromeContentBrowserClientExtensionsPart;

namespace {

bool HandleURLRewrite(GURL* url,
content::BrowserContext* browser_context) {
if (url->SchemeIs(content::kChromeUIScheme) &&
(url->host() == chrome::kChromeUIWelcomeHost ||
url->host() == chrome::kChromeUIWelcomeWin10Host)) {
*url = GURL(kBraveUIWelcomeURL);
bool HandleURLOverrideRewrite(GURL* url,
content::BrowserContext* browser_context) {
// redirect sync-internals
if (url->host() == chrome::kChromeUISyncInternalsHost ||
url->host() == chrome::kChromeUISyncHost) {
Copy link
Member

@simonhong simonhong Jan 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think this second condition in if clause can be removed.

GURL::Replacements replacements;
replacements.SetHostStr(chrome::kChromeUISyncHost);
*url = url->ReplaceComponents(replacements);
return true;
}
if (url->SchemeIs(content::kChromeUIScheme) &&
(url->host() == kBraveUISyncHost)) {
*url = GURL(kBraveUISyncURL);

// no special win10 welcome page
if (url->host() == chrome::kChromeUIWelcomeWin10Host ||
url->host() == chrome::kChromeUIWelcomeHost) {
*url = GURL(chrome::kChromeUIWelcomeURL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q) Why do you make rest handler skip for these two hosts (sync and welcome) by returning true?
Do you think upstream url handler doesn't need to handle them?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! I saw upstream code tries to rewritten it. Ignore my above comment.

return true;
}

return false;
}

bool HandleURLReverseRewrite(GURL* url,

bool HandleURLReverseOverrideRewrite(GURL* url,
content::BrowserContext* browser_context) {
if (url->spec() == kBraveUIWelcomeURL ||
url->spec() == kBraveUISyncURL) {
if (url->host() == chrome::kChromeUIWelcomeHost ||
url->host() == chrome::kChromeUISyncHost) {
GURL::Replacements replacements;
replacements.SetSchemeStr(kBraveUIScheme);
*url = url->ReplaceComponents(replacements);
return true;
}

return false;
}

bool HandleURLRewrite(GURL* url,
content::BrowserContext* browser_context) {
if (url->SchemeIs(kBraveUIScheme)) {
GURL::Replacements replacements;
replacements.SetSchemeStr(content::kChromeUIScheme);
*url = url->ReplaceComponents(replacements);
}

if (HandleURLOverrideRewrite(url, browser_context))
return true;

return false;
}

} // namespace

BraveContentBrowserClient::BraveContentBrowserClient(ChromeFeatureListCreator* chrome_feature_list_creator) :
ChromeContentBrowserClient(chrome_feature_list_creator)
{}
BraveContentBrowserClient::BraveContentBrowserClient(
ChromeFeatureListCreator* chrome_feature_list_creator)
: ChromeContentBrowserClient(chrome_feature_list_creator) {}

BraveContentBrowserClient::~BraveContentBrowserClient() {}

content::BrowserMainParts* BraveContentBrowserClient::CreateBrowserMainParts(
const content::MainFunctionParams& parameters) {
ChromeBrowserMainParts* main_parts = (ChromeBrowserMainParts*)
ChromeContentBrowserClient::CreateBrowserMainParts(parameters);
ChromeBrowserMainParts* main_parts = static_cast<ChromeBrowserMainParts*>(
ChromeContentBrowserClient::CreateBrowserMainParts(parameters));
main_parts->AddParts(new BraveBrowserMainExtraParts());
return main_parts;
}
Expand All @@ -110,12 +138,18 @@ void BraveContentBrowserClient::BrowserURLHandlerCreated(
handler->AddHandlerPair(&webtorrent::HandleTorrentURLRewrite,
&webtorrent::HandleTorrentURLReverseRewrite);
handler->AddHandlerPair(&HandleURLRewrite,
&HandleURLReverseRewrite);
&HandleURLReverseOverrideRewrite);
ChromeContentBrowserClient::BrowserURLHandlerCreated(handler);
handler->AddHandlerPair(&HandleURLOverrideRewrite,
&HandleURLReverseOverrideRewrite);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add new pair after adding super class' handlers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the order is critical because of rewriting that the superclass does

}

bool BraveContentBrowserClient::AllowAccessCookie(const GURL& url, const GURL& first_party,
content::ResourceContext* context, int render_process_id, int render_frame_id) {
bool BraveContentBrowserClient::AllowAccessCookie(
const GURL& url,
const GURL& first_party,
content::ResourceContext* context,
int render_process_id,
int render_frame_id) {
GURL tab_origin =
BraveShieldsWebContentsObserver::GetTabURLFromRenderFrameInfo(
render_process_id, render_frame_id, -1).GetOrigin();
Expand Down Expand Up @@ -169,7 +203,7 @@ BraveContentBrowserClient::AllowWebBluetooth(
content::BrowserContext* browser_context,
const url::Origin& requesting_origin,
const url::Origin& embedding_origin) {
return content::ContentBrowserClient::AllowWebBluetoothResult::BLOCK_GLOBALLY_DISABLED;
return ContentBrowserClient::AllowWebBluetoothResult::BLOCK_GLOBALLY_DISABLED;
}

bool BraveContentBrowserClient::HandleExternalProtocol(
Expand All @@ -188,7 +222,8 @@ bool BraveContentBrowserClient::HandleExternalProtocol(
}

return ChromeContentBrowserClient::HandleExternalProtocol(
url, web_contents_getter, child_id, navigation_data, is_main_frame, page_transition, has_user_gesture, method, headers);
url, web_contents_getter, child_id, navigation_data, is_main_frame,
page_transition, has_user_gesture, method, headers);
}

void BraveContentBrowserClient::RegisterOutOfProcessServices(
Expand Down Expand Up @@ -217,7 +252,6 @@ BraveContentBrowserClient::GetNavigationUIData(
TorProfileServiceFactory::SetTorNavigationUIData(profile,
navigation_ui_data.get());
return std::move(navigation_ui_data);

}

std::unique_ptr<base::Value>
Expand Down Expand Up @@ -285,11 +319,14 @@ void BraveContentBrowserClient::MaybeHideReferrer(
GURL(),
CONTENT_SETTINGS_TYPE_PLUGINS,
brave_shields::kBraveShields);
brave_shields::ShouldSetReferrer(allow_referrers, shields_up,
referrer->url, document_url, request_url,
request_url.GetOrigin(),
referrer->policy,
referrer);
brave_shields::ShouldSetReferrer(allow_referrers,
shields_up,
referrer->url,
document_url,
request_url,
request_url.GetOrigin(),
referrer->policy,
referrer);
}

GURL BraveContentBrowserClient::GetEffectiveURL(
Expand Down
139 changes: 116 additions & 23 deletions browser/brave_content_browser_client_browsertest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
/* 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/. */

Expand All @@ -19,6 +20,7 @@
#include "chrome/browser/ui/browser_window.h"
#include "chrome/common/buildflags.h"
#include "chrome/common/chrome_content_client.h"
#include "chrome/common/webui_url_constants.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
Expand Down Expand Up @@ -114,36 +116,127 @@ class BraveContentBrowserClientTest : public InProcessBrowserTest {
};

IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, CanLoadChromeURL) {
GURL chrome_settings_url("chrome://about/");
std::vector<GURL> urls{chrome_settings_url, GURL("about:about/"),
GURL("brave://about/")};
std::for_each(urls.begin(), urls.end(),
[this, &chrome_settings_url](const GURL& url) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
ui_test_utils::NavigateToURL(browser(), url);
ASSERT_TRUE(WaitForLoadStop(contents));
EXPECT_STREQ(contents->GetLastCommittedURL().spec().c_str(),
chrome_settings_url.spec().c_str());
});
std::vector<std::string> pages {
chrome::kChromeUIWelcomeHost,
};

std::vector<std::string> schemes {
"about:",
"brave://",
"chrome://",
};

for (const std::string& page : pages) {
for (const std::string& scheme : schemes) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
ui_test_utils::NavigateToURL(browser(), GURL(scheme + page + "/"));
ASSERT_TRUE(WaitForLoadStop(contents));

EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
->GetVirtualURL().spec().c_str(),
("brave://" + page + "/").c_str());
EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
->GetURL().spec().c_str(),
("chrome://" + page + "/").c_str());
}
}
}

IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, CanLoadCustomBravePages) {
std::vector<GURL> urls {
GURL("chrome://adblock/"),
std::vector<std::string> pages {
"adblock",
#if BUILDFLAG(BRAVE_REWARDS_ENABLED)
GURL("chrome://rewards/"),
"rewards",
#endif
GURL("chrome://welcome/"), GURL("chrome://sync/")
chrome::kChromeUISyncHost,
};

std::vector<std::string> schemes {
"brave://",
"chrome://",
};

for (const std::string& page : pages) {
for (const std::string& scheme : schemes) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
ui_test_utils::NavigateToURL(browser(), GURL(scheme + page + "/"));
ASSERT_TRUE(WaitForLoadStop(contents));

EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
->GetVirtualURL().spec().c_str(),
("brave://" + page + "/").c_str());
EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
->GetURL().spec().c_str(),
("chrome://" + page + "/").c_str());
}
}
}

IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, CanLoadAboutHost) {
std::vector<std::string> schemes {
"chrome://",
"brave://",
};

for (const std::string& scheme : schemes) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
ui_test_utils::NavigateToURL(browser(), GURL(scheme + "about/"));
ASSERT_TRUE(WaitForLoadStop(contents));

EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
->GetVirtualURL().spec().c_str(),
"brave://about/");
EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
->GetURL().spec().c_str(),
"chrome://chrome-urls/");
}
}

IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest,
RewriteChromeSyncInternals) {
std::vector<std::string> schemes {
"brave://",
"chrome://",
};

for (const std::string& scheme : schemes) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
ui_test_utils::NavigateToURL(browser(), GURL(scheme + "sync-internals/"));
ASSERT_TRUE(WaitForLoadStop(contents));

EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
->GetVirtualURL().spec().c_str(),
"brave://sync/");
EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
->GetURL().spec().c_str(),
"chrome://sync/");
}
}

IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest,
RewriteChromeWelcomeWin10) {
std::vector<std::string> schemes {
"brave://",
"chrome://",
};
std::for_each(urls.begin(), urls.end(), [this](const GURL& url) {

for (const std::string& scheme : schemes) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
ui_test_utils::NavigateToURL(browser(), url);
ui_test_utils::NavigateToURL(browser(), GURL(scheme + "welcome-win10/"));
ASSERT_TRUE(WaitForLoadStop(contents));
EXPECT_STREQ(contents->GetLastCommittedURL().spec().c_str(),
url.spec().c_str());
});

EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
->GetVirtualURL().spec().c_str(),
"brave://welcome/");
EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
->GetURL().spec().c_str(),
"chrome://welcome/");
}
}

IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, RewriteMagnetURLURLBar) {
Expand Down Expand Up @@ -327,7 +420,7 @@ IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest,

class BraveContentBrowserClientReferrerTest
: public BraveContentBrowserClientTest {
public:
public:
HostContentSettingsMap* content_settings() {
return HostContentSettingsMapFactory::GetForProfile(browser()->profile());
}
Expand Down
8 changes: 5 additions & 3 deletions browser/ui/webui/brave_web_ui_controller_factory.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
/* 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 "brave/browser/ui/webui/brave_web_ui_controller_factory.h"

#include <memory>

#include "brave/browser/ui/webui/brave_adblock_ui.h"
#include "brave/browser/ui/webui/brave_md_settings_ui.h"
#include "brave/browser/ui/webui/brave_new_tab_ui.h"
Expand Down Expand Up @@ -70,7 +73,7 @@ WebUIFactoryFunction GetWebUIFactoryFunction(WebUI* web_ui,
url.host_piece() == kDonateHost ||
#endif
url.host_piece() == kWelcomeHost ||
url.host_piece() == kBraveUIWelcomeURL ||
url.host_piece() == chrome::kChromeUIWelcomeURL ||
(url.host_piece() == kBraveUISyncHost &&
brave_sync::BraveSyncService::is_enabled()) ||
url.host_piece() == chrome::kChromeUINewTabHost ||
Expand All @@ -85,7 +88,6 @@ WebUIFactoryFunction GetWebUIFactoryFunction(WebUI* web_ui,

WebUI::TypeID BraveWebUIControllerFactory::GetWebUIType(
content::BrowserContext* browser_context, const GURL& url) const {

WebUIFactoryFunction function = GetWebUIFactoryFunction(NULL, url);
if (function) {
return reinterpret_cast<WebUI::TypeID>(function);
Expand Down
Loading