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

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Nika Layzell [:nika] (ni? for response)]

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.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow, r=

MozReview-Commit-ID: AjbVZHEL9WZ

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Add a way to track Constellations (Units of related similar origin browsing contexts) to nsGlobalWindow, r=

MozReview-Commit-ID: AjbVZHEL9WZ

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

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?

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: 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

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Attachment: 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)

Comment 6

[Nika Layzell [:nika] (ni? for response)]

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?

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 8

[Nika Layzell [:nika] (ni? for response)]

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?

Comment 9

[Nika Layzell [:nika] (ni? for response)]

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

MozReview-Commit-ID: AjbVZHEL9WZ

Comment 10

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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?

Comment 11

[Nika Layzell [:nika] (ni? for response)]

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

Updated

Comment 12

[Pulsebot]

Pushed by [e-mail]:
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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e03332b2656b

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

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