863515 - Electrolysis: Make content.opener work

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P3)

Description

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

The browser code frequently checks if content.opener is non-null. We should be able to forward this information from the content process.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

This seems to magically work due to CPOWs - ie, the scratchpad in e10s builds tells me content.opener is "[object XrayWrapper [object Window]]" and I can fetch properties on it (eg, content.opener.location.href is correct, etc).

Is there something more subtle I'm missing?

Comment 2

[Tom S [:evilpie]]

That works, but is of course not really nice, because we have to create all these wrappers just for such a simple check.

Comment 3

[Chris Peterson [:cpeterson]]

Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".

Comment 4

[Chris Peterson [:cpeterson]]

Moving to M3 milestone. These bugs affect e10s dogfooding but Brad and Gavin did not think they were M2 blockers.

Comment 5

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

Moving this up to m5+. This doesn't really have much user-visible effect at all.

Comment 6

[Chris Peterson [:cpeterson]]

m5+

Comment 7

[Mike Conley (:mconley) (:⚙️)]

I took this, but having read it, I don't think it qualifies as an M5. Re-nomming.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: bz://863515/mconley

/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=?
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=?

Pull down these commits:

hg pull -r a8b9112c5d9954ceebbece2a28dbaba403f89c16 https://reviewboard-hg.mozilla.org/gecko/

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=?
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=?

Pull down these commits:

hg pull -r a8b9112c5d9954ceebbece2a28dbaba403f89c16 https://reviewboard-hg.mozilla.org/gecko/

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d0b016b2fe1

Comment 11

[:Felipe Gomes (needinfo for replies!)]

https://reviewboard.mozilla.org/r/8419/#review7109

When a tab is navigated to a different page, the content.opener information will change, right? So the cached value must be updated. Here the initial value for a tab will remain throughout its existence.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

When I test single-process Firefox, tabs with openers (like one opened from target="_blank") seem to keep that opener for the lifetime of the outer browser window... do you see that too?

I was trying to emulate that behaviour. I've never even read the spec behind opener - not sure what the "right" thing to do here is.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=?
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=?

Pull down these commits:

hg pull -r d7d1c65a573aef2f27a5d67c627c0e958b77938f https://reviewboard-hg.mozilla.org/gecko/

Comment 14

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

https://reviewboard.mozilla.org/r/8417/#review7203

::: browser/base/content/browser.js:4174
(Diff revisions 1 - 2)
> +        Pocket.onLocationChange(browser, aLocationURI);

is this part of the patch?

Comment 15

[Mike Conley (:mconley) (:⚙️)]

https://reviewboard.mozilla.org/r/8417/#review7261

> is this part of the patch?

Nope, this Review Board not understanding that this patch series was rebased. Sorry about that. :)

Comment 16

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

https://reviewboard.mozilla.org/r/8417/#review7263

Ship It!

Comment 17

[Mike Conley (:mconley) (:⚙️)]

https://reviewboard.mozilla.org/r/8417/#review7297

Pretty depressing results on my try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e544939e244

I don't think this patch is going to cut it. :(

Comment 18

[Mike Conley (:mconley) (:⚙️)]

Ok, I've gone through my notes on how we do window opening and tab opening, and I think I know how we can pull this off more safely. New patch coming up.

Comment 19

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8723 - Bug 863515 - Expose hasContentOpener on nsITabParent. r=?
/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=?
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=?

Pull down these commits:

hg pull -r 73cd053d96985c5cb1667a8821a419f3a6641600 https://reviewboard-hg.mozilla.org/gecko/

Comment 20

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8723 - Bug 863515 - Expose hasContentOpener on nsITabParent. r=?
/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=?
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=?

Pull down these commits:

hg pull -r 73cd053d96985c5cb1667a8821a419f3a6641600 https://reviewboard-hg.mozilla.org/gecko/

Comment 21

[:Felipe Gomes (needinfo for replies!)]

https://reviewboard.mozilla.org/r/8421/#review7445

Ship It!

Comment 22

[:Felipe Gomes (needinfo for replies!)]

https://reviewboard.mozilla.org/r/8419/#review7447

Ship It!

Comment 23

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8723 - Bug 863515 - Expose hasContentOpener on nsITabParent. r=?
/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=felipe.
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=felipe.

Pull down these commits:

hg pull -r e53fb43afdd10ca6d8dff970abdaa82cb0848ca7 https://reviewboard-hg.mozilla.org/gecko/

Comment 24

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

update iid of nsITabParent

Comment 25

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/fcee8fe97ec8
https://hg.mozilla.org/integration/fx-team/rev/fa82c160f259
https://hg.mozilla.org/integration/fx-team/rev/b8965a7ca29d

Comment 26

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

https://hg.mozilla.org/mozilla-central/rev/fcee8fe97ec8
https://hg.mozilla.org/mozilla-central/rev/fa82c160f259
https://hg.mozilla.org/mozilla-central/rev/b8965a7ca29d

Comment 28

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 863515 - Expose hasContentOpener on nsITabParent. r=?

Comment 29

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=felipe.

Comment 30

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=felipe.

Comment 31

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bugnotes

http://people.mozilla.org/~mconley2/bugnotes/bug-863515.html