Remove the PBrowser::Msg_GetWidgetNativeData sync IPC

RESOLVED FIXED in Firefox 56

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Ehsan, Assigned: Nika)

Tracking

(Blocks 1 bug)

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [qf:p1])

Attachments

(1 attachment, 3 obsolete attachments)

Reporter

Description

2 years ago
This is used in order to give plugins a handle to the native widget.  It seems easy to pass this handle down to the TabChild at creation time, but we should probably do that after bug 1343728 lands.  That way, it should be super simple to remove this sync IPC.
Whiteboard: [qf]
Whiteboard: [qf] → [qf:p1]

Comment 1

2 years ago
Jim, can you confirm that once we're windowless-only we don't need this handle any more?
Flags: needinfo?(jmathies)
(In reply to Benjamin Smedberg [:bsmedberg] from comment #1)
> Jim, can you confirm that once we're windowless-only we don't need this
> handle any more?

I don't think we can remove windowed support as long as users have the ability to disable Flash async drawing. Also, we probably have a large population of facebook game users who have this off, remember Zynga was advertising to users to disable it for a while on at least one popular facebook game.

We could change the pref name maybe, but I don't think we should remove the ability to flip a pref to test the old windowed mode until we're sure async drawing is going to stick and work for everyone. I'm open to being convinced otherwise though, I'd love to rip out a ton of windowed mode plugin code.
Flags: needinfo?(jmathies) → needinfo?(benjamin)

Comment 3

2 years ago
I do want to aggressively rip out the windowed-mode support, but probably have async drawing on release for a release cycle first.

But in this case the question is specifically around prioritizing QF work. If we only need/send this message with windowed-mode plugins, then this is "fixed" with async drawing.
Flags: needinfo?(benjamin)
Asking billm for review on the changes to sync-messages.

bz - I am not sure who should review this change - but I saw you had reviewed some code which touched this code, so I'm asking you.

MozReview-Commit-ID: Bql29wgVDZ5
Attachment #8877237 - Flags: review?(wmccloskey)
Attachment #8877237 - Flags: review?(bzbarsky)
This patch doesn't depend on bug 1343728, although I have written it on top of the same tree, so I'm removing the dependency.
Assignee: nobody → michael
No longer blocks: qf-bugs-upforgrabs
Fixed some try failures.

MozReview-Commit-ID: Bql29wgVDZ5
Attachment #8877303 - Flags: review?(wmccloskey)
Attachment #8877303 - Flags: review?(bzbarsky)
Assignee

Updated

2 years ago
Attachment #8877237 - Attachment is obsolete: true
Attachment #8877237 - Flags: review?(wmccloskey)
Attachment #8877237 - Flags: review?(bzbarsky)
Attachment #8877303 - Flags: review?(wmccloskey) → review+
Comment on attachment 8877303 [details] [diff] [review]
Remove the Msg_GetWidgetNativeData sync IPC

Going to try punting to jimm, but please punt back if you're not happy reviewing this.  Then I'll try to figure out this stuff.
Attachment #8877303 - Flags: review?(bzbarsky) → review?(jmathies)
Comment on attachment 8877303 [details] [diff] [review]
Remove the Msg_GetWidgetNativeData sync IPC

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

::: dom/ipc/TabChild.h
@@ +891,5 @@
>    bool mPendingDocShellPreserveLayers;
>    bool mPendingDocShellReceivedMessage;
>    uint32_t mPendingDocShellBlockers;
>  
> +  // The HANDLE object for the widget this TabChild in.

nit - I'd probably put this comment up on WidgetNativeData()
Attachment #8877303 - Flags: review?(jmathies) → review+
MozReview-Commit-ID: Bql29wgVDZ5
MozReview-Commit-ID: Bql29wgVDZ5
Assignee

Updated

2 years ago
Attachment #8877303 - Attachment is obsolete: true
Attachment #8878630 - Attachment is obsolete: true

Comment 11

2 years ago
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/956306ea34f3
Remove the Msg_GetWidgetNativeData sync IPC, r=billm, r=jimm
Backed out bug 1343728 and bug 1350633 for failing various tests, e.g. mochitest test_animations_reverse.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/105d12cd1d6d72f4f3a52584c909bff34e8e43fa
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e927a18b19c9b48bcd8e5b83895b078bbc051ba
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e623a352cbb73651b3a18549f3096247cb310ad
https://hg.mozilla.org/integration/mozilla-inbound/rev/42ce4647768307b20114899b8c3de8c622791342

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=956306ea34f36514350ce6f03c114eedbabe934f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure example: https://treeherder.mozilla.org/logviewer.html#?job_id=107791530&repo=mozilla-inbound
[task 2017-06-16T19:32:30.922114Z] 19:32:30     INFO - TEST-START | layout/style/test/test_animations_reverse.html
[task 2017-06-16T19:32:31.502503Z] 19:32:31     INFO - TEST-INFO | started process screentopng
[task 2017-06-16T19:32:33.217196Z] 19:32:33     INFO - TEST-INFO | screentopng: exit 0
[task 2017-06-16T19:32:33.220930Z] 19:32:33     INFO - TEST-UNEXPECTED-FAIL | layout/style/test/test_animations_reverse.html | OMTA should work 
[task 2017-06-16T19:32:33.222139Z] 19:32:33     INFO -     runOMTATest/<@layout/style/test/animation_utils.js:217:9
[task 2017-06-16T19:32:33.223374Z] 19:32:33     INFO -     promise callback*runOMTATest@layout/style/test/animation_utils.js:209:3
[task 2017-06-16T19:32:33.224574Z] 19:32:33     INFO -     @layout/style/test/file_animations_reverse.html:36:1
Flags: needinfo?(michael)

Comment 13

2 years ago
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/36d662fbab77
Remove the Msg_GetWidgetNativeData sync IPC, r=billm, r=jimm
I had to back this out because it or something else from the push caused bug 1375293, which was failing more frequently than we could handle.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/79c8da654a9301f57f4c9216eeb2ca1c76d58603

Comment 15

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/36d662fbab77
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56

Comment 16

2 years ago
Backout by philringnalda@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/8a11785cd3f2
Backed out changeset 36d662fbab77 for failures in test_group_mouseevents.html a=backout
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla56 → ---

Comment 17

2 years ago
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1e773337202
Remove the Msg_GetWidgetNativeData sync IPC, r=billm, r=jimm

Comment 18

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a1e773337202
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
backed out because of test failures after a incomplete backout see https://bugzilla.mozilla.org/show_bug.cgi?id=1375940#c19 and #20
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 21

2 years ago
Backout by cbook@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/8307aba1cf5c
Backed out changeset a1e773337202

Comment 22

2 years ago
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/85fb6e3ceb7a
Remove the Msg_GetWidgetNativeData sync IPC, r=billm, r=jimm
Assignee

Updated

2 years ago
Flags: needinfo?(michael)

Comment 23

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/85fb6e3ceb7a
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.