Electrolysis: Make content.opener work

RESOLVED FIXED in Firefox 41

Status

()

defect
P3
normal
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: billm, Assigned: mconley)

Tracking

(Blocks 1 bug)

unspecified
Firefox 41
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm7+, firefox41 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

The browser code frequently checks if content.opener is non-null. We should be able to forward this information from the content process.
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?
That works, but is of course not really nice, because we have to create all these wrappers just for such a simple check.
Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".
tracking-e10s: --- → +
Blocks: 691608
Assignee: nobody → wmccloskey
Priority: -- → P3
No longer blocks: old-e10s-m2
Whiteboard: [e10s-m3]
Moving to M3 milestone. These bugs affect e10s dogfooding but Brad and Gavin did not think they were M2 blockers.
Moving this up to m5+. This doesn't really have much user-visible effect at all.
m5+
Whiteboard: [e10s-m3]
Assignee

Updated

4 years ago
Assignee: wmccloskey → mconley
Assignee

Updated

4 years ago
Status: NEW → ASSIGNED
I took this, but having read it, I don't think it qualifies as an M5. Re-nomming.
Status: ASSIGNED → NEW
/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 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/
Attachment #8603393 - Flags: review?(felipc)
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.
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 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 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?
Attachment #8603393 - Flags: review?(felipc)
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 on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

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

Ship It!
Attachment #8603393 - Flags: review+
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. :(
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.
Assignee

Updated

4 years ago
Attachment #8603393 - Flags: review+ → review?(felipc)
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 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/
Attachment #8603393 - Flags: review?(bugs)
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/
Attachment #8603393 - Flags: review?(felipc)
Attachment #8603393 - Flags: review?(bugs) → review+
update iid of nsITabParent
Attachment #8603393 - Attachment is obsolete: true
Attachment #8618011 - Flags: review+
Attachment #8618012 - Flags: review+
Attachment #8618013 - Flags: review+
You need to log in before you can comment on or make changes to this bug.