1335678 - Add diagnostic assertions to see if OriginAttributes and loadGroup are in sync

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P3)

Description

[Andrea Marchesini [:baku]]

+++ This bug was initially created as a clone of Bug #1302566 +++

Comment 1

[Andrea Marchesini [:baku]]

Attachment: revert.patch

Comment 2

[(no longer active)]

Comment on attachment 8832374 [details] [diff] [review]
revert.patch

Review of attachment 8832374 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsContentUtils.cpp
@@ +1812,5 @@
> +    return isPrivate;
> +  }
> +
> +  nsCOMPtr<nsIChannel> channel = aDoc->GetChannel();
> +  return channel && NS_UsePrivateBrowsing(channel);

It's not really great that we're now exposing a different way of getting this info from nsContentUtils.

I think what you want instead is to make this a private static function, and expose a VerifyDocumentPrivateBrowsingState() that will compute the information both using the OA from the principal and the helper mentioned above, and asserts they're the same.

Comment 3

[Tanvi Vyas[:tanvi]]

[Tracking Requested - why for this release]:
https://bugzilla.mozilla.org/show_bug.cgi?id=1302566 landed in Firefox 54.  We should have the assertions that validate that that conversion happens properly in the same release.  Otherwise, we risk introducing a bug in private mode / regular mode separation.

Comment 4

[Andrea Marchesini [:baku]]

Attachment: revert.patch

Comment 5

[Tanvi Vyas[:tanvi]]

baku said when he pushed this to try, he got lots of failures.  So he is going to back out https://bugzilla.mozilla.org/show_bug.cgi?id=1302566 after all.

Comment 6

[Marcia Knous [:marcia]]

Tracking 54+ for the reasons in Comment 3.

Comment 7

[Andrea Marchesini [:baku]]

Attachment: revert_a.patch

The previous patch broke everything. We need to revert the commit and see why there are so many failures with these assertions.

Comment 8

[(no longer active)]

Comment on attachment 8833918 [details] [diff] [review]
revert_a.patch

Review of attachment 8833918 [details] [diff] [review]:
-----------------------------------------------------------------

Sadface :(

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c00dba4cfdf
Revert patch for bug 1302566, r=ehsan

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7c00dba4cfdf