1124245 - [E10s] IsParentActivated() call in nsGlobalWindow looks wrong

Status: RESOLVED FIXED

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

Description

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

...since IsParentActivated() relies on global state in nsFocusManager, and the same
nsFocusManager can be used for several tabs in several parent process windows.

Comment 1

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

In fact we can't really keep mParentFocusType as such in nsFocusManager.

Comment 2

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

(/me kicks himself for not catching this while reviewing the patch.)

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Smaug, what do you want to do here?

Comment 4

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

Store the state in TabChild and nsGlobalWindow should read the state when it gets its docshell.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

What's the effect of this? Is there any user-visible bug here?

Comment 6

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

Yes, but let me fix this.

Comment 7

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

Attachment: v1

nsFocusManager::WindowShown is still a bit odd, but decided to not change
the current behavior.

https://tbpl.mozilla.org/?tree=Try&rev=897aca538dba

Comment 8

[Neil Deakin]

Comment on attachment 8555566 [details] [diff] [review]
v1

>-    mRemoteBrowser->Show(size);
>+    nsPIDOMWindow* win = mOwnerContent->OwnerDoc()->GetWindow();
>+    bool parentIsActive = false;
>+    if (win) {
>+      nsCOMPtr<nsPIWindowRoot> windowRoot =
>+        static_cast<nsGlobalWindow*>(win)->GetTopWindowRoot();
>+      if (windowRoot) {
>+        nsPIDOMWindow* topWin = windowRoot->GetWindow();
>+        parentIsActive = topWin && topWin->IsActive();
>+      }
>+    }

The active state gets propagated down to child windows. Do you need to use the root window here?

Comment 9

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

We don't propagate that flag. We propagate NS_DOCUMENT_STATE_WINDOW_INACTIVE state.

Comment 10

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

Boo, there is some bug in the patch. It doesn't update background tabs properly or something.

Comment 11

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

Attachment: v2

This passes locally.
(I'm still not quite happy how we deal with activate, especially in non-e10s case, but that is for some other bug)

https://tbpl.mozilla.org/?tree=Try&rev=a2e570a4abfd

The change to the test is needed just to be able to run it separately locally on linux.

interdiff coming

Comment 12

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

Attachment: interdiff 1 -> 2

Comment 13

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/647d24c9a40a

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/647d24c9a40a