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.

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

16256: Properly get tab origin for brave shields. #9078

Merged
merged 1 commit into from
Jun 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 35 additions & 0 deletions browser/brave_shields/ad_block_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,41 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, SubFrame) {
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 2ULL);
}

// Checks nothing is blocked if shields are off.
IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, SubFrameShieldsOff) {
ASSERT_TRUE(InstallDefaultAdBlockExtension());

EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
GURL url = embedded_test_server()->GetURL("a.com", "/iframe_blocking.html");

brave_shields::SetBraveShieldsEnabled(content_settings(), false, url);

ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();

EXPECT_EQ(true, EvalJs(contents->GetAllFrames()[1],
"setExpectations(0, 0, 1, 0);"
"xhr('adbanner.js?1')"));
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);

// Check also an explicit request for a script since it is a common real-world
// scenario.
EXPECT_EQ(true, EvalJs(contents->GetAllFrames()[1],
R"(
new Promise(function (resolve, reject) {
var s = document.createElement('script');
s.onload = () => resolve(true);
s.onerror = reject;
s.src = 'adbanner.js?2';
document.head.appendChild(s);
})
)"));
content::RunAllTasksUntilIdle();
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
brave_shields::ResetBraveShieldsEnabled(content_settings(), url);
}

// Requests made by a service worker should be blocked as well.
IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, ServiceWorkerRequest) {
UpdateAdBlockInstanceWithRules("adbanner.js");
Expand Down
8 changes: 5 additions & 3 deletions browser/net/url_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,11 @@ std::shared_ptr<brave::BraveRequestInfo> BraveRequestInfo::MakeCTX(
// |AddChannelRequest| provides only old-fashioned |site_for_cookies|.
// (See |BraveProxyingWebSocket|).
if (ctx->tab_origin.is_empty()) {
ctx->tab_origin = brave_shields::BraveShieldsWebContentsObserver::
GetTabURLFromRenderFrameInfo(ctx->frame_tree_node_id)
.GetOrigin();
content::WebContents* contents =
content::WebContents::FromFrameTreeNodeId(ctx->frame_tree_node_id);
if (contents) {
ctx->tab_origin = contents->GetLastCommittedURL().GetOrigin();
}
}

if (old_ctx) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@

#include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h"

#include <map>
#include <memory>
#include <string>
#include <utility>
#include <vector>

#include "base/strings/utf_string_conversions.h"
#include "brave/common/pref_names.h"
Expand Down Expand Up @@ -47,7 +45,6 @@ using extensions::Event;
using extensions::EventRouter;
#endif

using content::Referrer;
using content::RenderFrameHost;
using content::WebContents;

Expand Down Expand Up @@ -84,11 +81,6 @@ void UpdateContentSettingsToRendererFrames(content::WebContents* web_contents) {

namespace brave_shields {

base::Lock BraveShieldsWebContentsObserver::frame_data_map_lock_;

std::map<int, GURL>
BraveShieldsWebContentsObserver::frame_tree_node_id_to_tab_url_;

BraveShieldsWebContentsObserver::~BraveShieldsWebContentsObserver() {
}

Expand All @@ -107,19 +99,9 @@ void BraveShieldsWebContentsObserver::RenderFrameCreated(
WebContents* web_contents = WebContents::FromRenderFrameHost(rfh);
if (web_contents) {
UpdateContentSettingsToRendererFrames(web_contents);

base::AutoLock lock(frame_data_map_lock_);
frame_tree_node_id_to_tab_url_[rfh->GetFrameTreeNodeId()] =
web_contents->GetURL();
}
}

void BraveShieldsWebContentsObserver::RenderFrameDeleted(
RenderFrameHost* rfh) {
base::AutoLock lock(frame_data_map_lock_);
frame_tree_node_id_to_tab_url_.erase(rfh->GetFrameTreeNodeId());
}

void BraveShieldsWebContentsObserver::RenderFrameHostChanged(
RenderFrameHost* old_host, RenderFrameHost* new_host) {
if (old_host) {
Expand All @@ -130,31 +112,6 @@ void BraveShieldsWebContentsObserver::RenderFrameHostChanged(
}
}

void BraveShieldsWebContentsObserver::DidFinishNavigation(
content::NavigationHandle* navigation_handle) {
RenderFrameHost* main_frame = web_contents()->GetMainFrame();
if (!web_contents() || !main_frame) {
return;
}
int tree_node_id = main_frame->GetFrameTreeNodeId();

base::AutoLock lock(frame_data_map_lock_);
frame_tree_node_id_to_tab_url_[tree_node_id] = web_contents()->GetURL();
}

// static
GURL BraveShieldsWebContentsObserver::GetTabURLFromRenderFrameInfo(
int render_frame_tree_node_id) {
base::AutoLock lock(frame_data_map_lock_);
if (-1 != render_frame_tree_node_id) {
auto iter = frame_tree_node_id_to_tab_url_.find(render_frame_tree_node_id);
if (iter != frame_tree_node_id_to_tab_url_.end()) {
return iter->second;
}
}
return GURL();
}

bool BraveShieldsWebContentsObserver::IsBlockedSubresource(
const std::string& subresource) {
return blocked_url_paths_.find(subresource) != blocked_url_paths_.end();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include <vector>

#include "base/macros.h"
#include "base/synchronization/lock.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h"

Expand Down Expand Up @@ -47,13 +46,10 @@ class BraveShieldsWebContentsObserver : public content::WebContentsObserver,
protected:
// content::WebContentsObserver overrides.
void RenderFrameCreated(content::RenderFrameHost* host) override;
void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
void RenderFrameHostChanged(content::RenderFrameHost* old_host,
content::RenderFrameHost* new_host) override;
void ReadyToCommitNavigation(
content::NavigationHandle* navigation_handle) override;
void DidFinishNavigation(
content::NavigationHandle* navigation_handle) override;

// Invoked if an IPC message is coming from a specific RenderFrameHost.
bool OnMessageReceived(const IPC::Message& message,
Expand All @@ -65,12 +61,6 @@ class BraveShieldsWebContentsObserver : public content::WebContentsObserver,
content::RenderFrameHost* render_frame_host,
const std::u16string& details);

// TODO(iefremov): Refactor this away or at least put into base::NoDestructor.
// Protects global maps below from being concurrently written on the UI thread
// and read on the IO thread.
static base::Lock frame_data_map_lock_;
static std::map<int, GURL> frame_tree_node_id_to_tab_url_;

private:
friend class content::WebContentsUserData<BraveShieldsWebContentsObserver>;
std::vector<std::string> allowed_script_origins_;
Expand Down