1199765 - Add support for querying active state of remote browsers

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Jim Mathies [:jimm]]

We need this so we can enable tests that check active state.

Comment 1

[Jim Mathies [:jimm]]

Attachment: cpp bits v.1

Adds caching for active state and support for returning the value from remote-browser.

Comment 2

[Jim Mathies [:jimm]]

Attachment: browser bits v.1

The scary thing about this is that without the tabbrowser fixes this test fails.

Comment 3

[Jim Mathies [:jimm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e41ee9f4a2cf

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8655478 [details] [diff] [review]
cpp bits v.1

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

::: dom/interfaces/base/nsITabParent.idl
@@ +29,5 @@
> +    * Returns the doc shell active state. Note this does not query the
> +    * actual docShell on the child side, it returns a value cached in
> +    * SetIsDocShellActive.
> +    */
> +  readonly attribute boolean docShellIsActive;

Any reason not to make this a read/write attribute and get rid of the method above?

Comment 5

[Jim Mathies [:jimm]]

(In reply to Dave Townsend [:mossop] from comment #4)
> Comment on attachment 8655478 [details] [diff] [review]
> cpp bits v.1
> 
> Review of attachment 8655478 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/interfaces/base/nsITabParent.idl
> @@ +29,5 @@
> > +    * Returns the doc shell active state. Note this does not query the
> > +    * actual docShell on the child side, it returns a value cached in
> > +    * SetIsDocShellActive.
> > +    */
> > +  readonly attribute boolean docShellIsActive;
> 
> Any reason not to make this a read/write attribute and get rid of the method
> above?

Yeah I can try that I guess. Let me search around, make sure I have a handle on all the uses of the older api.

Comment 6

[Jim Mathies [:jimm]]

Doesn't look bad, and I get zero hits in addons code.

Comment 7

[Jim Mathies [:jimm]]

Attachment: cpp bits v.2

Updated some naming, so requesting follow up review.

Comment 8

[Jim Mathies [:jimm]]

Attachment: cpp bits v.2

- updated a comment.

Comment 9

[Dave Townsend [:mossop]]

Comment on attachment 8655594 [details] [diff] [review]
cpp bits v.2

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

Didn't spot the name collision there, still this looks good

Comment 10

[Jim Mathies [:jimm]]

Attachment: browser bits v.2

Expanded version of this test that includes checking to make sure the cached c++ value is correct.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/542c2121b3b1
https://hg.mozilla.org/integration/mozilla-inbound/rev/cdcb00107c17

Comment 12

[Jim Mathies [:jimm]]

The push didn't include the tabbrowser.xml changes. It turns out that having two tabs active is by design with e10s, we manage active state through the tabswitcher defined in tabbrowser. Basically we need to turn the new tab on so that it will paint, and keep the old tab active briefly since we keep it visible until layer tree updates arrive. So the second patch just updates the test.

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/542c2121b3b1
https://hg.mozilla.org/mozilla-central/rev/cdcb00107c17