Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow

RESOLVED FIXED in Firefox 51

Status

()

Core
DOM
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: mystor, Assigned: mystor)

Tracking

(Blocks: 1 bug)

unspecified
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment, 5 obsolete attachments)

This is a simple patch which splits the idea into the "related" (aka static constellation) and "similar origin" (can be converted into each other by document.domain) browsing contexts. Each set of related browsing contexts is assigned an static constellation ID, and the constellation ID is generated when requested from this ID and the current document's principal's URI's ELTD domain.
Created attachment 8790499 [details] [diff] [review]
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow, r=

MozReview-Commit-ID: AjbVZHEL9WZ
Created attachment 8790500 [details] [diff] [review]
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow, r=

MozReview-Commit-ID: AjbVZHEL9WZ
Attachment #8790499 - Attachment is obsolete: true
Is it not possible to just trace through parent and opener in order to find the root of a constellation?  Is this to allow checks like "are these two windows in the same constellation" without tracing?
Created attachment 8790566 [details] [diff] [review]
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow, r=

Good point, this one just walks the tree when you ask for it, and caches the result because it shouldn't change.

I'm not sure if I'm using the APIs very well for walking that tree - I might have screwed it up :S
Attachment #8790500 - Attachment is obsolete: true
Created attachment 8790567 [details] [diff] [review]
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow, r=

Oops, apparently forgot to add the changes to the commit (/facepalm it's late)
Attachment #8790566 - Attachment is obsolete: true
Comment on attachment 8790567 [details] [diff] [review]
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow, r=

Is this sort-of what you were thinking of billm?
Attachment #8790567 - Flags: feedback?(wmccloskey)
Comment on attachment 8790567 [details] [diff] [review]
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow, r=

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

This looks good. One concern I have about the lazy approach is if you have three windows like this:

W1: opener=null
W2: opener=W1
W3: opener=W2

Now you close W2. I think that W2's opener will be set to null. If you compute the constellation now, it will look like W3 and W1 are in different constellations. However, W3 could have saved a reference to W1 while W2 was still alive.

I think we could fix this by saving the opener window ID when we set the opener.

::: dom/base/nsGlobalWindow.cpp
@@ +14398,5 @@
> +    nsCOMPtr<nsPIDOMWindowOuter> window = AsOuter();
> +    while (true) {
> +      nsCOMPtr<nsPIDOMWindowOuter> parent = window->GetParent();
> +      if (parent != window) {
> +        window = parent.forget();

I think you're kind of inlining GetTop here.
Attachment #8790567 - Flags: feedback?(wmccloskey) → feedback+
Comment on attachment 8790500 [details] [diff] [review]
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow, r=

Is this older patch more what you were thinking of?
Attachment #8790500 - Attachment is obsolete: false
Attachment #8790500 - Flags: feedback?(wmccloskey)
Blocks: 1302270
Created attachment 8790937 [details] [diff] [review]
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow

MozReview-Commit-ID: AjbVZHEL9WZ
Attachment #8790937 - Flags: review?(wmccloskey)
(Assignee)

Updated

11 months ago
Attachment #8790500 - Attachment is obsolete: true
Attachment #8790500 - Flags: feedback?(wmccloskey)
(Assignee)

Updated

11 months ago
Attachment #8790567 - Attachment is obsolete: true
Comment on attachment 8790937 [details] [diff] [review]
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow

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

::: dom/base/nsGlobalWindow.cpp
@@ +3036,5 @@
>    }
>  
>    mDocShell = aDocShell; // Weak Reference
>  
> +  // Copy over the static constellation from our new parent

Period at the end of the sentence.

@@ +14406,5 @@
> +void
> +nsGlobalWindow::GetConstellation(nsACString& aConstellation)
> +{
> +  FORWARD_TO_INNER_VOID(GetConstellation, (aConstellation));
> +

Can we assert at the top of this function that GetTop() and GetOpener() have the same static constellation as we do?
Attachment #8790937 - Flags: review?(wmccloskey) → review+
Created attachment 8791277 [details] [diff] [review]
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow

Updated
(Assignee)

Updated

11 months ago
Attachment #8790937 - Attachment is obsolete: true

Comment 12

11 months ago
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e03332b2656b
Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow, r=billm

Comment 13

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e03332b2656b
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51

Comment 14

11 months ago
(bikeshedding, but I think having static constellation and constellation makes it hard to understand which one means what. I'd prefer 'galaxy' for static constellation, but there has been also some other name proposals.)
(Assignee)

Updated

11 months ago
Blocks: 1303196
You need to log in before you can comment on or make changes to this bug.