16256: Properly get tab origin for brave shields.

Fixes brave/brave-browser#16265
brave · Jun 8, 2021 · 5ba14dd · 5ba14dd
1 parent 3d64f2c
commit 5ba14dd
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 40 deletions.
diff --git a/browser/brave_shields/ad_block_service_browsertest.cc b/browser/brave_shields/ad_block_service_browsertest.cc
@@ -505,6 +505,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");

diff --git a/browser/brave_shields/brave_shields_web_contents_observer.cc b/browser/brave_shields/brave_shields_web_contents_observer.cc
@@ -5,13 +5,10 @@
 
 #include "brave/browser/brave_shields/brave_shields_web_contents_observer.h"
 
-#include <map>
 #include <memory>
 #include <string>
 #include <utility>
-#include <vector>
 
-#include "base/no_destructor.h"
 #include "base/strings/utf_string_conversions.h"
 #include "brave/common/pref_names.h"
 #include "brave/components/brave_perf_predictor/browser/buildflags.h"
@@ -48,7 +45,6 @@ using extensions::Event;
 using extensions::EventRouter;
 #endif
 
-using content::Referrer;
 using content::RenderFrameHost;
 using content::WebContents;
 
@@ -84,8 +80,6 @@ void UpdateContentSettingsToRendererFrames(content::WebContents* web_contents) {
 
 namespace brave_shields {
 
-base::NoDestructor<std::map<int, GURL>> frame_tree_node_id_to_tab_url_;
-
 BraveShieldsWebContentsObserver::~BraveShieldsWebContentsObserver() {
   brave_shields_remotes_.clear();
 }
@@ -104,14 +98,10 @@ void BraveShieldsWebContentsObserver::RenderFrameCreated(RenderFrameHost* rfh) {
   WebContents* web_contents = WebContents::FromRenderFrameHost(rfh);
   if (web_contents) {
     UpdateContentSettingsToRendererFrames(web_contents);
-
-    (*frame_tree_node_id_to_tab_url_)[rfh->GetFrameTreeNodeId()] =
-        web_contents->GetURL();
   }
 }
 
 void BraveShieldsWebContentsObserver::RenderFrameDeleted(RenderFrameHost* rfh) {
-  (*frame_tree_node_id_to_tab_url_).erase(rfh->GetFrameTreeNodeId());
   brave_shields_remotes_.erase(rfh);
 }
 
@@ -126,31 +116,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();
-
-  (*frame_tree_node_id_to_tab_url_)[tree_node_id] = web_contents()->GetURL();
-}
-
-// static
-GURL BraveShieldsWebContentsObserver::GetTabURLFromRenderFrameInfo(
-    int render_frame_tree_node_id) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (render_frame_tree_node_id != RenderFrameHost::kNoFrameTreeNodeId) {
-    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();

diff --git a/browser/brave_shields/brave_shields_web_contents_observer.h b/browser/brave_shields/brave_shields_web_contents_observer.h
@@ -57,8 +57,6 @@ class BraveShieldsWebContentsObserver
                               content::RenderFrameHost* new_host) override;
   void ReadyToCommitNavigation(
       content::NavigationHandle* navigation_handle) override;
-  void DidFinishNavigation(
-      content::NavigationHandle* navigation_handle) override;
 
   // brave_shields::mojom::BraveShieldsHost.
   void OnJavaScriptBlocked(const std::u16string& details) override;

diff --git a/browser/net/url_context.cc b/browser/net/url_context.cc
@@ -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) {