The condition here is when the network.cookie.cookieBehavior pref is set to 1.

Now without my local preferences changes

Comment on attachment 8606449 [details] [diff] [review] v1 Review of attachment 8606449 [details] [diff] [review]: ----------------------------------------------------------------- You need to ask the review to some docshell peer. smaug or bz maybe? ::: docshell/base/nsDocShell.cpp @@ +14024,5 @@ > + Preferences::GetBool("network.cookie.cookieBehavior", true); > + > + bool isThirdPartyURI = true; > + thirdPartyUtil->IsThirdPartyURI(mCurrentURI, aURI, &isThirdPartyURI); > + if (isThirdPartyURI && disallowThirdPartyUrls) { if (isThirdPartyURI && // Check pref to see if we should prevent frameset spoofing Preferences::GetBool("network.cookie.cookieBehavior", true)) { return NS_OK; }

Comment on attachment 8606449 [details] [diff] [review] v1 >+ // Check pref to see if we should prevent frameset spoofing >+ bool disallowThirdPartyUrls = >+ Preferences::GetBool("network.cookie.cookieBehavior", true); network.cookie.cookieBehavior is not a bool pref Did you test this all using different network.cookie.cookieBehavior values?

Comment on attachment 8613179 [details] [diff] [review] v2 Try isn't looking good with this patch https://treeherder.mozilla.org/#/jobs?repo=try&revision=32623a44e14d

Try looks better now https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fa9a7df380a

Comment on attachment 8614164 [details] [diff] [review] v3 >+#define COOKIE_BEHAVIOR_REJECTFOREIGN 1 perhaps _ between REJECT and FOREIGN >+ nsresult rv; >+ nsCOMPtr<mozIThirdPartyUtil> thirdPartyUtil = >+ do_GetService(THIRDPARTYUTIL_CONTRACTID, &rv); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ if (mCurrentURI) { >+ // Reject the interception of third-party iframes if the cookie behaviour >+ // is set to reject all third-party cookies. >+ bool isThirdPartyURI = true; >+ thirdPartyUtil->IsThirdPartyURI(mCurrentURI, aURI, &isThirdPartyURI); >+ if (isThirdPartyURI && >+ (Preferences::GetInt("network.cookie.cookieBehavior", >+ COOKIE_BEHAVIOR_REJECTFOREIGN) == >+ COOKIE_BEHAVIOR_REJECTFOREIGN)) { pass 0 as the default, not 1 (COOKIE_BEHAVIOR_REJECTFOREIGN), since all.js defaults to 0 too. And perhaps add a comment why 0 But someone familiar with ServiceWorker implementation must review this too, and check the tests too. I'm really not familiar enough with SW. So perhaps baku.

Comment on attachment 8614164 [details] [diff] [review] v3 Review of attachment 8614164 [details] [diff] [review]: ----------------------------------------------------------------- ::: docshell/base/nsDocShell.cpp @@ +201,5 @@ > #include "nsIBrowserSearchService.h" > #endif > > +#include "mozIThirdPartyUtil.h" > +#define COOKIE_BEHAVIOR_REJECTFOREIGN 1 Write a comment about what this '1' means. Or a link to the documentation, or a link to some code. @@ +14056,5 @@ > + if (mCurrentURI) { > + // Reject the interception of third-party iframes if the cookie behaviour > + // is set to reject all third-party cookies. > + bool isThirdPartyURI = true; > + thirdPartyUtil->IsThirdPartyURI(mCurrentURI, aURI, &isThirdPartyURI); This can fail. ::: dom/workers/test/serviceworkers/test_third_party_iframes.html @@ +23,5 @@ > +let index = 0; > +function next() { > + info("Step " + index); > + if (index >= steps.length) { > + ok(false, "Shouldn't get here!"); SimpleTest.finish(); @@ +40,5 @@ > + ["dom.serviceWorkers.exemptFromPerDomainMax", true], > + ["dom.serviceWorkers.enabled", true], > + ["dom.serviceWorkers.testing.enabled", true], > + ["browser.dom.window.dump.enabled", true] > + ]}, next); Move this in the steps and do: onload = next(); of course, in next do: steps[index++](); @@ +167,5 @@ > + SpecialPowers.pushPrefEnv({"set": [ > + ["network.cookie.cookieBehavior", COOKIE_BEHAVIOR_LIMITFOREIGN] > + ]}, next); > +}, () => { > + testShouldIntercept(SimpleTest.finish); call next here too.