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

RESOLVED FIXED in Firefox 51

Status

()

defect
RESOLVED FIXED
3 years ago
a month ago

People

(Reporter: Nika, Assigned: Nika)

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.
(Assignee)

Updated

3 years ago
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?
(Assignee)

Comment 4

3 years ago
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
(Assignee)

Updated

3 years ago
Attachment #8790500 - Attachment is obsolete: true
(Assignee)

Comment 5

3 years ago
Oops, apparently forgot to add the changes to the commit (/facepalm it's late)
(Assignee)

Updated

3 years ago
Attachment #8790566 - Attachment is obsolete: true
(Assignee)

Comment 6

3 years ago
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+
(Assignee)

Comment 8

3 years ago
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)

Updated

3 years ago
Blocks: QuantumDOM
(Assignee)

Comment 9

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

Updated

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

Updated

3 years 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+
(Assignee)

Updated

3 years ago
Attachment #8790937 - Attachment is obsolete: true

Comment 12

3 years 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

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e03332b2656b
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
(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

3 years ago
Blocks: 1303196
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.