16256: Properly get tab origin for brave shields.

browser/brave_shields/ad_block_service_browsertest.cc

@@ -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");

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) {

components/brave_shields/browser/brave_shields_web_contents_observer.cc

@@ -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"
@@ -47,7 +45,6 @@ using extensions::Event;
 using extensions::EventRouter;
 #endif
 
-using content::Referrer;
 using content::RenderFrameHost;
 using content::WebContents;
 
@@ -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() {
 }
 
@@ -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) {
@@ -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();

components/brave_shields/browser/brave_shields_web_contents_observer.h

@@ -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"
 
@@ -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,
@@ -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_;