1350633 - Remove the PBrowser::Msg_GetWidgetNativeData sync IPC

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, enhancement)

Description

[(no longer active)]

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.

Comment 1

[Benjamin Smedberg]

Jim, can you confirm that once we're windowless-only we don't need this handle any more?

Comment 2

[Jim Mathies [:jimm]]

(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.

Comment 3

[Benjamin Smedberg]

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.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Remove the Msg_GetWidgetNativeData sync IPC

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

Comment 5

[Nika Layzell [:nika] (ni? for response)]

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.

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Remove the Msg_GetWidgetNativeData sync IPC

Fixed some try failures.

MozReview-Commit-ID: Bql29wgVDZ5

Comment 7

[Boris Zbarsky [:bzbarsky]]

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.

Comment 8

[Jim Mathies [:jimm]]

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()

Comment 9

[Nika Layzell [:nika] (ni? for response)]

Attachment: Remove the Msg_GetWidgetNativeData sync IPC, r=billm

MozReview-Commit-ID: Bql29wgVDZ5

Comment 10

[Nika Layzell [:nika] (ni? for response)]

Attachment: Remove the Msg_GetWidgetNativeData sync IPC

MozReview-Commit-ID: Bql29wgVDZ5

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/956306ea34f3
Remove the Msg_GetWidgetNativeData sync IPC, r=billm, r=jimm

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/36d662fbab77
Remove the Msg_GetWidgetNativeData sync IPC, r=billm, r=jimm

Comment 14

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

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

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

https://hg.mozilla.org/mozilla-central/rev/36d662fbab77

Comment 16

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/8a11785cd3f2
Backed out changeset 36d662fbab77 for failures in test_group_mouseevents.html a=backout

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1e773337202
Remove the Msg_GetWidgetNativeData sync IPC, r=billm, r=jimm

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a1e773337202

Comment 19

[Carsten Book [:Tomcat]]

backed out because of test failures after a incomplete backout see https://bugzilla.mozilla.org/show_bug.cgi?id=1375940#c19 and #20

Comment 20

[Kartikaya Gupta (email:[e-mail])]

Backout push is here: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=cc903e3f61894e60c3b0efaf05e9a446d1d85888

Comment 21

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/8307aba1cf5c
Backed out changeset a1e773337202

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/85fb6e3ceb7a
Remove the Msg_GetWidgetNativeData sync IPC, r=billm, r=jimm

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/85fb6e3ceb7a