Closed Bug 1069059 Opened 7 years ago Closed 7 years ago

[e10s] Convert more isWindowPrivate callers

Categories

(Firefox :: Private Browsing, defect)

34 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 35
Tracking Status
e10s m3+ ---

People

(Reporter: billm, Assigned: billm)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(1 file)

Attached patch pb-fixesSplinter Review
This is a follow-up to bug 1066447. I fixed more callers and added an isContentWindowPrivate variant for content scripts.

It would be nice if we could throw in isWindowPrivate if the argument is not a chrome window, but I'm not confident enough to do that yet. Let's let the warning sit for a while first.
Attachment #8491188 - Flags: review?(ttaubert)
Comment on attachment 8491188 [details] [diff] [review]
pb-fixes

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

There's a whole load of mobile/ changes as well, let's ask a mobile peer for review too.

::: toolkit/modules/PrivateBrowsingUtils.jsm
@@ +20,5 @@
>    // isBrowserPrivate since it works with e10s.
>    isWindowPrivate: function pbu_isWindowPrivate(aWindow) {
> +    if (!(aWindow instanceof Components.interfaces.nsIDOMChromeWindow)) {
> +      dump("WARNING: content window passed to PrivateBrowsingUtils.isWindowPrivate. " +
> +           "Use isContentWindowPrivate instead.\n"

Can we mention that isContentWindowPrivate() should only be used by frame scripts?
Attachment #8491188 - Flags: review?(ttaubert)
Attachment #8491188 - Flags: review?(margaret.leibovic)
Attachment #8491188 - Flags: review+
Comment on attachment 8491188 [details] [diff] [review]
pb-fixes

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

Thanks for the heads up, Tim. The /mobile changes look good to me.
Attachment #8491188 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/324798b60ba3
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Depends on: 1073826
Depends on: 1074410
Depends on: 1075000
Flags: qe-verify-
Added dev-doc-needed because this introduced PrivateBrowsingUtils.isContentWindowPrivate() and deprecated PrivateBrowsingUtils.isWindowPrivate() for content windows - yet the documentation (meaning only https://developer.mozilla.org/en-US/docs/Supporting_per-window_private_browsing from what I can see) is unaware of that.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.