Issue 2095: use the proper tab origin.

brave/brave-browser#2095

As per URLRequest documentation and the corresponding RFC,
site_for_cookies cannot be used as a top-level document origin since it
can be empty in certain cases. We need to replace it with something more
reliable.

browser/brave_content_browser_client.cc

@@ -97,7 +97,7 @@ bool BraveContentBrowserClient::AllowAccessCookie(const GURL& url, const GURL& f
     content::ResourceContext* context, int render_process_id, int render_frame_id) {
   GURL tab_origin =
       BraveShieldsWebContentsObserver::GetTabURLFromRenderFrameInfo(
-          render_process_id, render_frame_id).GetOrigin();
+          render_process_id, render_frame_id, -1).GetOrigin();
   ProfileIOData* io_data = ProfileIOData::FromResourceContext(context);
   bool allow_brave_shields =
       brave_shields::IsAllowContentSettingWithIOData(

browser/net/brave_ad_block_tp_network_delegate_helper.cc

@@ -121,11 +121,11 @@ void OnBeforeURLRequestDispatchOnIOThread(
     ctx->new_url_spec != ctx->request_url.spec()) {
     if (ctx->blocked_by == kAdBlocked) {
       brave_shields::DispatchBlockedEventFromIO(ctx->request_url,
-          ctx->render_process_id, ctx->render_frame_id, ctx->frame_tree_node_id,
+          ctx->render_frame_id, ctx->render_process_id, ctx->frame_tree_node_id,
           brave_shields::kAds);
     } else if (ctx->blocked_by == kTrackerBlocked) {
       brave_shields::DispatchBlockedEventFromIO(ctx->request_url,
-          ctx->render_process_id, ctx->render_frame_id, ctx->frame_tree_node_id,
+          ctx->render_frame_id, ctx->render_process_id, ctx->frame_tree_node_id,
           brave_shields::kTrackers);
     }
   }

browser/net/brave_httpse_network_delegate_helper.cc

@@ -33,7 +33,7 @@ void OnBeforeURLRequest_HttpsePostFileWork(
   if (!ctx->new_url_spec.empty() &&
     ctx->new_url_spec != ctx->request_url.spec()) {
     brave_shields::DispatchBlockedEventFromIO(ctx->request_url,
-        ctx->render_process_id, ctx->render_frame_id, ctx->frame_tree_node_id,
+        ctx->render_frame_id, ctx->render_process_id, ctx->frame_tree_node_id,
         brave_shields::kHTTPUpgradableResources);
   }
 
@@ -79,7 +79,7 @@ int OnBeforeURLRequest_HttpsePreFileWork(
     } else {
       if (!ctx->new_url_spec.empty()) {
         brave_shields::DispatchBlockedEventFromIO(ctx->request_url,
-            ctx->render_process_id, ctx->render_frame_id,
+            ctx->render_frame_id, ctx->render_process_id,
             ctx->frame_tree_node_id,
             brave_shields::kHTTPUpgradableResources);
       }

browser/net/brave_network_delegate_base.cc

@@ -9,6 +9,7 @@
 #include "base/task/post_task.h"
 #include "brave/common/pref_names.h"
 #include "brave/components/brave_shields/browser/brave_shields_util.h"
+#include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h"
 #include "brave/components/brave_shields/common/brave_shield_constants.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
@@ -27,13 +28,13 @@ using net::URLRequest;
 namespace {
 
   content::WebContents* GetWebContentsFromProcessAndFrameId(
-      int render_process_id, int render_frame_id) {
+      int render_process_id, int render_frame_id, int frame_tree_node_id) {
   if (render_process_id) {
     content::RenderFrameHost* rfh =
         content::RenderFrameHost::FromID(render_process_id, render_frame_id);
     return content::WebContents::FromRenderFrameHost(rfh);
   }
-  return content::WebContents::FromFrameTreeNodeId(render_frame_id);
+  return content::WebContents::FromFrameTreeNodeId(frame_tree_node_id);
 }
 
 }  // namespace
@@ -160,14 +161,11 @@ bool BraveNetworkDelegateBase::OnCanGetCookies(const URLRequest& request,
         return callback.Run(ctx);
       });
 
-  int frame_id;
-  int process_id;
-  int frame_tree_node_id;
-  brave_shields::GetRenderFrameInfo(&request, &frame_id, &process_id,
-      &frame_tree_node_id);
   base::RepeatingCallback<content::WebContents*(void)> wc_getter =
-      base::BindRepeating(&GetWebContentsFromProcessAndFrameId, process_id,
-                          frame_id);
+      base::BindRepeating(&GetWebContentsFromProcessAndFrameId,
+                          ctx->render_process_id,
+                          ctx->render_frame_id,
+                          ctx->frame_tree_node_id);
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::UI},
       base::BindOnce(&TabSpecificContentSettings::CookiesRead, wc_getter,
@@ -185,19 +183,17 @@ bool BraveNetworkDelegateBase::OnCanSetCookie(const URLRequest& request,
       new brave::BraveRequestInfo());
   brave::BraveRequestInfo::FillCTXFromRequest(&request, ctx);
   ctx->event_type = brave::kOnCanSetCookies;
+
   bool allow = std::all_of(can_set_cookies_callbacks_.begin(), can_set_cookies_callbacks_.end(),
       [&ctx](brave::OnCanSetCookiesCallback callback){
         return callback.Run(ctx);
       });
 
-  int frame_id;
-  int process_id;
-  int frame_tree_node_id;
-  brave_shields::GetRenderFrameInfo(&request, &frame_id, &process_id,
-      &frame_tree_node_id);
   base::RepeatingCallback<content::WebContents*(void)> wc_getter =
-      base::BindRepeating(&GetWebContentsFromProcessAndFrameId, process_id,
-                          frame_id);
+      base::BindRepeating(&GetWebContentsFromProcessAndFrameId,
+                          ctx->render_process_id,
+                          ctx->render_frame_id,
+                          ctx->frame_tree_node_id);
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::UI},
       base::BindOnce(&TabSpecificContentSettings::CookieChanged, wc_getter,

browser/net/brave_network_delegate_browsertest.cc

@@ -0,0 +1,101 @@
+/* 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 "base/path_service.h"
+#include "brave/common/brave_paths.h"
+#include "brave/components/brave_shields/common/brave_shield_constants.h"
+#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.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"
+#include "content/public/test/browser_test_utils.h"
+#include "net/dns/mock_host_resolver.h"
+#include "url/gurl.h"
+
+class BraveNetworkDelegateBrowserTest : public InProcessBrowserTest {
+ public:
+  void SetUpOnMainThread() override {
+    InProcessBrowserTest::SetUpOnMainThread();
+
+    host_resolver()->AddRule("*", "127.0.0.1");
+    content::SetupCrossSiteRedirector(embedded_test_server());
+
+    brave::RegisterPathProvider();
+    base::FilePath test_data_dir;
+    base::PathService::Get(brave::DIR_TEST_DATA, &test_data_dir);
+    embedded_test_server()->ServeFilesFromDirectory(test_data_dir);
+
+    ASSERT_TRUE(embedded_test_server()->Start());
+
+    url_ = embedded_test_server()->GetURL("a.com", "/nested_iframe.html");
+    nested_iframe_script_url_ =
+        embedded_test_server()->GetURL("a.com", "/nested_iframe_script.html");
+
+    top_level_page_pattern_ =
+        ContentSettingsPattern::FromString("http://a.com/*");
+    first_party_pattern_ =
+        ContentSettingsPattern::FromString("https://firstParty/*");
+  }
+
+  HostContentSettingsMap* content_settings() {
+    return HostContentSettingsMapFactory::GetForProfile(browser()->profile());
+  }
+
+  void AllowCookies() {
+    content_settings()->SetContentSettingCustomScope(
+        top_level_page_pattern_, ContentSettingsPattern::Wildcard(),
+        CONTENT_SETTINGS_TYPE_PLUGINS, brave_shields::kCookies,
+        CONTENT_SETTING_ALLOW);
+    content_settings()->SetContentSettingCustomScope(
+        top_level_page_pattern_, first_party_pattern_,
+        CONTENT_SETTINGS_TYPE_PLUGINS, brave_shields::kCookies,
+        CONTENT_SETTING_ALLOW);
+  }
+
+ protected:
+  GURL url_;
+  GURL nested_iframe_script_url_;
+
+ private:
+  ContentSettingsPattern top_level_page_pattern_;
+  ContentSettingsPattern first_party_pattern_;
+  ContentSettingsPattern iframe_pattern_;
+};
+
+// It is important that cookies in following tests are set by response headers,
+// not by javascript. Fetching such cookies is controlled by NetworkDelegate.
+IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest, Iframe3PCookieBlocked) {
+  ui_test_utils::NavigateToURL(browser(), url_);
+  const std::string cookie =
+      content::GetCookies(browser()->profile(), GURL("http://c.com/"));
+  EXPECT_TRUE(cookie.empty()) << "Actual cookie: " << cookie;
+}
+
+IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest, Iframe3PCookieAllowed) {
+  AllowCookies();
+  ui_test_utils::NavigateToURL(browser(), url_);
+  const std::string cookie =
+      content::GetCookies(browser()->profile(), GURL("http://c.com/"));
+  EXPECT_FALSE(cookie.empty());
+}
+
+// Fetching not just a frame, but some other resource.
+IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
+                       IframeJs3PCookieBlocked) {
+  ui_test_utils::NavigateToURL(browser(), nested_iframe_script_url_);
+  const std::string cookie =
+      content::GetCookies(browser()->profile(), GURL("http://c.com/"));
+  EXPECT_TRUE(cookie.empty()) << "Actual cookie: " << cookie;
+}
+
+IN_PROC_BROWSER_TEST_F(BraveNetworkDelegateBrowserTest,
+                       IframeJs3PCookieAllowed) {
+  AllowCookies();
+  ui_test_utils::NavigateToURL(browser(), nested_iframe_script_url_);
+  const std::string cookie =
+      content::GetCookies(browser()->profile(), GURL("http://c.com/"));
+  EXPECT_FALSE(cookie.empty());
+}

browser/net/url_context.cc

@@ -9,6 +9,7 @@
 
 #include "brave/common/url_constants.h"
 #include "brave/components/brave_shields/browser/brave_shields_util.h"
+#include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h"
 #include "brave/components/brave_shields/common/brave_shield_constants.h"
 #include "content/public/browser/resource_request_info.h"
 
@@ -24,13 +25,25 @@ void BraveRequestInfo::FillCTXFromRequest(const net::URLRequest* request,
     std::shared_ptr<brave::BraveRequestInfo> ctx) {
   ctx->request_identifier = request->identifier();
   ctx->request_url = request->url();
-  ctx->tab_origin = request->site_for_cookies().GetOrigin();
   auto* request_info = content::ResourceRequestInfo::ForRequest(request);
   if (request_info) {
     ctx->resource_type = request_info->GetResourceType();
   }
-  brave_shields::GetRenderFrameInfo(request, &ctx->render_process_id, &ctx->render_frame_id,
-      &ctx->frame_tree_node_id);
+  brave_shields::GetRenderFrameInfo(request,
+                                    &ctx->render_frame_id,
+                                    &ctx->render_process_id,
+                                    &ctx->frame_tree_node_id);
+  if (!request->site_for_cookies().is_empty()) {
+    ctx->tab_url = request->site_for_cookies();
+  } else {
+    // We can not always use site_for_cookies since it can be empty in certain
+    // cases. See the comments in url_request.h
+    ctx->tab_url = brave_shields::BraveShieldsWebContentsObserver::
+        GetTabURLFromRenderFrameInfo(ctx->render_process_id,
+                                     ctx->render_frame_id,
+                                     ctx->frame_tree_node_id).GetOrigin();
+  }
+  ctx->tab_origin = ctx->tab_url.GetOrigin();
   ctx->allow_brave_shields = brave_shields::IsAllowContentSettingFromIO(
       request, ctx->tab_origin, ctx->tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
       brave_shields::kBraveShields) &&
@@ -50,5 +63,4 @@ void BraveRequestInfo::FillCTXFromRequest(const net::URLRequest* request,
   ctx->request = request;
 }
 
-
 }  // namespace brave

browser/net/url_context.h

@@ -51,6 +51,7 @@ struct BraveRequestInfo {
   ~BraveRequestInfo();
   GURL request_url;
   GURL tab_origin;
+  GURL tab_url;
   std::string new_url_spec;
   bool allow_brave_shields = true;
   bool allow_ads = false;

components/brave_shields/browser/brave_shields_util.cc

@@ -107,9 +107,9 @@ void GetRenderFrameInfo(const URLRequest* request,
   if (request_info) {
     *frame_tree_node_id = request_info->GetFrameTreeNodeId();
   }
-  extensions::ExtensionApiFrameIdMap::FrameData frame_data;
   if (!content::ResourceRequestInfo::GetRenderFrameForRequest(
           request, render_process_id, render_frame_id)) {
+
     const content::WebSocketHandshakeRequestInfo* websocket_info =
       content::WebSocketHandshakeRequestInfo::ForRequest(request);
     if (websocket_info) {

components/brave_shields/browser/brave_shields_web_contents_observer.cc

@@ -94,7 +94,9 @@ namespace brave_shields {
 
 base::Lock BraveShieldsWebContentsObserver::frame_data_map_lock_;
 std::map<BraveShieldsWebContentsObserver::RenderFrameIdKey, GURL>
-    BraveShieldsWebContentsObserver::render_frame_key_to_tab_url;
+    BraveShieldsWebContentsObserver::frame_key_to_tab_url_;
+std::map<int, GURL>
+    BraveShieldsWebContentsObserver::frame_tree_node_id_to_tab_url_;
 
 BraveShieldsWebContentsObserver::RenderFrameIdKey::RenderFrameIdKey()
     : render_process_id(content::ChildProcessHost::kInvalidUniqueID),
@@ -136,27 +138,21 @@ void BraveShieldsWebContentsObserver::RenderFrameCreated(
   WebContents* web_contents = WebContents::FromRenderFrameHost(rfh);
   if (web_contents) {
     UpdateContentSettingsToRendererFrames(web_contents);
+
     base::AutoLock lock(frame_data_map_lock_);
     const RenderFrameIdKey key(rfh->GetProcess()->GetID(), rfh->GetRoutingID());
-    std::map<RenderFrameIdKey, GURL>::iterator iter =
-        render_frame_key_to_tab_url.find(key);
-    if (iter != render_frame_key_to_tab_url.end()) {
-      render_frame_key_to_tab_url.erase(key);
-    }
-    render_frame_key_to_tab_url.insert(
-        std::pair<RenderFrameIdKey, GURL>(key, web_contents->GetURL()));
+    frame_key_to_tab_url_[key] = web_contents->GetURL();
+    frame_tree_node_id_to_tab_url_[rfh->GetFrameTreeNodeId()] =
+        web_contents->GetURL();
   }
 }
 
 void BraveShieldsWebContentsObserver::RenderFrameDeleted(
     RenderFrameHost* rfh) {
   base::AutoLock lock(frame_data_map_lock_);
   const RenderFrameIdKey key(rfh->GetProcess()->GetID(), rfh->GetRoutingID());
-  std::map<RenderFrameIdKey, GURL>::iterator iter =
-      render_frame_key_to_tab_url.find(key);
-  if (iter != render_frame_key_to_tab_url.end()) {
-    render_frame_key_to_tab_url.erase(key);
-  }
+  frame_key_to_tab_url_.erase(key);
+  frame_tree_node_id_to_tab_url_.erase(rfh->GetFrameTreeNodeId());
 }
 
 void BraveShieldsWebContentsObserver::RenderFrameHostChanged(
@@ -169,15 +165,37 @@ void BraveShieldsWebContentsObserver::RenderFrameHostChanged(
   }
 }
 
+void BraveShieldsWebContentsObserver::DidFinishNavigation(
+    content::NavigationHandle* navigation_handle) {
+  RenderFrameHost* main_frame = web_contents()->GetMainFrame();
+  if (!web_contents() || !main_frame) {
+    return;
+  }
+  int process_id = main_frame->GetProcess()->GetID();
+  int routing_id = main_frame->GetRoutingID();
+  int tree_node_id = main_frame->GetFrameTreeNodeId();
+
+  base::AutoLock lock(frame_data_map_lock_);
+  frame_key_to_tab_url_[{process_id, routing_id}] = web_contents()->GetURL();
+  frame_tree_node_id_to_tab_url_[tree_node_id] = web_contents()->GetURL();
+}
+
 // static
 GURL BraveShieldsWebContentsObserver::GetTabURLFromRenderFrameInfo(
-    int render_process_id, int render_frame_id) {
+    int render_process_id, int render_frame_id, int render_frame_tree_node_id) {
   base::AutoLock lock(frame_data_map_lock_);
-  const RenderFrameIdKey key(render_process_id, render_frame_id);
-  std::map<RenderFrameIdKey, GURL>::iterator iter =
-      render_frame_key_to_tab_url.find(key);
-  if (iter != render_frame_key_to_tab_url.end()) {
-    return iter->second;
+  if (-1 != render_process_id && -1 != render_frame_id) {
+    auto iter = frame_key_to_tab_url_.find({render_process_id,
+                                            render_frame_id});
+    if (iter != frame_key_to_tab_url_.end()) {
+      return iter->second;
+    }
+  }
+  if (-1 != render_frame_tree_node_id) {
+    auto iter2 = frame_tree_node_id_to_tab_url_.find(render_frame_tree_node_id);
+    if (iter2 != frame_tree_node_id_to_tab_url_.end()) {
+      return iter2->second;
+    }
   }
   return GURL();
 }

components/brave_shields/browser/brave_shields_web_contents_observer.h

@@ -35,7 +35,9 @@ class BraveShieldsWebContentsObserver : public content::WebContentsObserver,
       std::string subresource,
       int render_process_id,
       int render_frame_id, int frame_tree_node_id);
-  static GURL GetTabURLFromRenderFrameInfo(int render_process_id, int render_frame_id);
+  static GURL GetTabURLFromRenderFrameInfo(int render_process_id,
+                                           int render_frame_id,
+                                           int render_frame_tree_node_id);
   void AllowScriptsOnce(const std::vector<std::string>& origins,
                         content::WebContents* web_contents);
   bool IsBlockedSubresource(const std::string& subresource);
@@ -64,6 +66,8 @@ class BraveShieldsWebContentsObserver : public content::WebContentsObserver,
                               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,
@@ -75,10 +79,12 @@ class BraveShieldsWebContentsObserver : public content::WebContentsObserver,
       content::RenderFrameHost* render_frame_host,
       const base::string16& details);
 
-  static std::map<RenderFrameIdKey, GURL> render_frame_key_to_tab_url;
-  // This lock protects |frame_data_map_| from being concurrently written on the
-  // UI thread and read on the IO thread.
+  // 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<RenderFrameIdKey, GURL> frame_key_to_tab_url_;
+  static std::map<int, GURL> frame_tree_node_id_to_tab_url_;
 
  private:
   friend class content::WebContentsUserData<BraveShieldsWebContentsObserver>;