Respond to review comments (dec15 21:43)

brave · Dec 15, 2020 · f02a83a · f02a83a
1 parent 3d9d684
commit f02a83a
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 17 deletions.
diff --git a/browser/ephemeral_storage/ephemeral_storage_browsertest.cc b/browser/ephemeral_storage/ephemeral_storage_browsertest.cc
@@ -116,19 +116,23 @@ class EphemeralStorageBrowserTest : public InProcessBrowserTest {
         content_settings, brave_shields::ControlType::ALLOW, GURL());
   }
 
+  void SetValuesInFrame(RenderFrameHost* frame,
+                        std::string storage_value,
+                        std::string cookie_value) {
+    SetStorageValueInFrame(frame, storage_value, StorageType::Local);
+    SetStorageValueInFrame(frame, storage_value, StorageType::Session);
+    SetCookieInFrame(frame, cookie_value);
+  }
+
   void SetValuesInFrames(WebContents* web_contents,
                          std::string storage_value,
                          std::string cookie_value) {
-    auto set_values_in_frame = [&](RenderFrameHost* frame) {
-      SetStorageValueInFrame(frame, storage_value, StorageType::Local);
-      SetStorageValueInFrame(frame, storage_value, StorageType::Session);
-      SetCookieInFrame(frame, cookie_value);
-    };
-
-    RenderFrameHost* main_frame = web_contents->GetMainFrame();
-    set_values_in_frame(main_frame);
-    set_values_in_frame(content::ChildFrameAt(main_frame, 0));
-    set_values_in_frame(content::ChildFrameAt(main_frame, 1));
+    RenderFrameHost* main = web_contents->GetMainFrame();
+    SetValuesInFrame(main, storage_value, cookie_value);
+    SetValuesInFrame(content::ChildFrameAt(main, 0), storage_value,
+                     cookie_value);
+    SetValuesInFrame(content::ChildFrameAt(main, 1), storage_value,
+                     cookie_value);
   }
 
   struct ValuesFromFrame {
@@ -476,7 +480,7 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest,
   auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents();
   RenderFrameHost* main_frame = web_contents->GetMainFrame();
   RenderFrameHost* iframe_a = content::ChildFrameAt(main_frame, 0);
-  RenderFrameHost* iframe_b = content::ChildFrameAt(main_frame, 0);
+  RenderFrameHost* iframe_b = content::ChildFrameAt(main_frame, 1);
   ASSERT_EQ("", GetCookiesInFrame(iframe_a));
   ASSERT_EQ("", GetCookiesInFrame(iframe_b));
 
@@ -503,3 +507,50 @@ IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest,
   EXPECT_EQ("", values_after.iframe_1.cookies);
   EXPECT_EQ("", values_after.iframe_2.cookies);
 }
+
+IN_PROC_BROWSER_TEST_F(EphemeralStorageBrowserTest,
+                       FirstPartyNestedInThirdParty) {
+  AllowAllCookies();
+
+  auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents();
+
+  GURL a_site_set_cookie_url = https_server_.GetURL(
+      "a.com", "/set-cookie?name=acom;path=/;SameSite=None;Secure");
+  ui_test_utils::NavigateToURL(browser(), a_site_set_cookie_url);
+  ui_test_utils::NavigateToURL(browser(), a_site_ephemeral_storage_url_);
+
+  RenderFrameHost* site_a_main_frame = web_contents->GetMainFrame();
+  RenderFrameHost* nested_frames_tab =
+      content::ChildFrameAt(site_a_main_frame, 3);
+  ASSERT_NE(nested_frames_tab, nullptr);
+  RenderFrameHost* first_party_nested_acom =
+      content::ChildFrameAt(nested_frames_tab, 2);
+  ASSERT_NE(first_party_nested_acom, nullptr);
+
+  WebContents* site_b_tab = LoadURLInNewTab(b_site_ephemeral_storage_url_);
+  RenderFrameHost* site_b_main_frame = site_b_tab->GetMainFrame();
+  RenderFrameHost* third_party_nested_acom =
+      content::ChildFrameAt(site_b_main_frame, 2);
+  ASSERT_NE(first_party_nested_acom, nullptr);
+
+  ASSERT_EQ("name=acom", GetCookiesInFrame(site_a_main_frame));
+  ASSERT_EQ("name=acom", GetCookiesInFrame(first_party_nested_acom));
+  ASSERT_EQ("", GetCookiesInFrame(third_party_nested_acom));
+
+  SetValuesInFrame(site_a_main_frame, "first-party-a.com",
+                   "name=first-party-a.com");
+  SetValuesInFrame(third_party_nested_acom, "third-party-a.com",
+                   "name=third-party-a.com");
+
+  ValuesFromFrame first_party_values =
+      GetValuesFromFrame(first_party_nested_acom);
+  EXPECT_EQ("first-party-a.com", first_party_values.local_storage);
+  EXPECT_EQ("first-party-a.com", first_party_values.session_storage);
+  EXPECT_EQ("name=first-party-a.com", first_party_values.cookies);
+
+  ValuesFromFrame third_party_values =
+      GetValuesFromFrame(third_party_nested_acom);
+  EXPECT_EQ("third-party-a.com", third_party_values.local_storage);
+  EXPECT_EQ("third-party-a.com", third_party_values.session_storage);
+  EXPECT_EQ("name=third-party-a.com", third_party_values.cookies);
+}
diff --git a/chromium_src/net/url_request/url_request_http_job.cc b/chromium_src/net/url_request/url_request_http_job.cc
@@ -5,8 +5,11 @@
 
 #include "net/url_request/url_request_http_job.h"
 
+#include "base/bind.h"
 #include "net/base/features.h"
+#include "net/base/isolation_info.h"
 #include "net/cookies/cookie_monster.h"
+#include "net/url_request/url_request.h"
 
 namespace {
 
@@ -28,26 +31,24 @@ bool ShouldUseEphemeralStorage(net::URLRequestHttpJob* http_job) {
 }  // namespace
 
 #define BRAVE_ADDCOOKIEHEADERANDSTART                                          \
-  if (ShouldUseEphemeralStorage(this)) {                                       \
-    DCHECK(request()->isolation_info().top_frame_origin().has_value());        \
+  if (ShouldUseEphemeralStorage(this))                                         \
     static_cast<CookieMonster*>(cookie_store)                                  \
         ->GetEphemeralCookieListWithOptionsAsync(                              \
             request_->url(),                                                   \
             request()->isolation_info().top_frame_origin()->GetURL(), options, \
             base::BindOnce(&URLRequestHttpJob::SetCookieHeaderAndStart,        \
                            weak_factory_.GetWeakPtr(), options));              \
-  } else  // NOLINT
+  else
 
 #define BRAVE_SAVECOOKIESANDNOTIFYHEADERSCOMPLETE                              \
-  if (ShouldUseEphemeralStorage(this)) {                                       \
-    DCHECK(request()->isolation_info().top_frame_origin().has_value());        \
+  if (ShouldUseEphemeralStorage(this))                                         \
     static_cast<CookieMonster*>(request_->context()->cookie_store())           \
         ->SetEphemeralCanonicalCookieAsync(                                    \
             std::move(cookie), request_->url(),                                \
             request()->isolation_info().top_frame_origin()->GetURL(), options, \
             base::BindOnce(&URLRequestHttpJob::OnSetCookieResult,              \
                            weak_factory_.GetWeakPtr(), options,                \
                            cookie_to_return, cookie_string));                  \
-  } else  // NOLINT
+  else
 
 #include "../../../../../net/url_request/url_request_http_job.cc"
diff --git a/test/data/ephemeral_storage.html b/test/data/ephemeral_storage.html
@@ -2,4 +2,6 @@
 <body>
 <iframe src="/cross-site/b.com/simple.html" id="third_party_iframe_a"></iframe>
 <iframe src="/cross-site/b.com/simple.html" id="third_party_iframe_b"></iframe>
+<iframe src="/cross-site/a.com/simple.html" id="first_party_iframe"></iframe>
+<iframe src="/cross-site/b.com/ephemeral_storage.html" id="nested_iframe"></iframe>
 </body></html>