1543173 - Turn off REQUIRE_DEFERRED_MESSAGE_PROTECTION in PluginModuleContentParent when native events are disabled

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, task, P2)

Description

[Jim Mathies [:jimm]]

Without native events, this flag should be removed. It enables code that allows for trapping native event messaging on window procedures during sync and rpc based incall waits in WindowsMessageLoop.

https://searchfox.org/mozilla-central/source/dom/plugins/ipc/PluginModuleParent.cpp#408
https://searchfox.org/mozilla-central/source/ipc/glue/WindowsMessageLoop.cpp#1075

Comment 1

[Jim Mathies [:jimm]]

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

Comment 2

[Jim Mathies [:jimm]]

Attachment: Bug 1543173 - Turn off REQUIRE_DEFERRED_MESSAGE_PROTECTION in PluginModuleContentParent when native events are disabled. r=bobowen

Comment 3

[Jim Mathies [:jimm]]

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

Comment 4

[Jim Mathies [:jimm]]

disable native events plus the theming work-
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4125e61a6f1a998cb557b23f3710c067fdae82a7

Comment 5

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #4)

disable native events plus the theming work-
https://treeherder.mozilla.org/#/
jobs?repo=try&revision=4125e61a6f1a998cb557b23f3710c067fdae82a7

Some theme related orange but nothing in plugins. looks good.

Comment 6

[Jim Mathies [:jimm]]

Attachment: Bug 1543173 - Turn off REQUIRE_DEFERRED_MESSAGE_PROTECTION in PluginModuleContentParent when native events are disabled. r=bobowen

Comment 7

[Jim Mathies [:jimm]]

For some background this code turns on deferred message handling in content. We introduced this because we would occasionally get caught in this situation:

1. content ipc plugin sync outcall -> plugin child process
2. during the processing of the child instance incall, flash process calls a system api that triggers sendmessage type event delivery to the plugin window hooked in parent [1]
3. this could trigger another out call to plugin instance child

[1] https://searchfox.org/mozilla-central/rev/dd7e27f4a805e4115d0dbee70e1220b23b23c567/dom/plugins/ipc/PluginInstanceParent.cpp#1872

Comment 8

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #7)

For some background this code turns on deferred message handling in content.
We introduced this because we would occasionally get caught in this
situation:

1. content ipc plugin sync outcall -> plugin child process
2. during the processing of the child instance incall, flash process calls a
   system api that triggers sendmessage type event delivery to the plugin
   window hooked in parent [1]
3. this could trigger another out call to plugin instance child

[1]
https://searchfox.org/mozilla-central/rev/
dd7e27f4a805e4115d0dbee70e1220b23b23c567/dom/plugins/ipc/
PluginInstanceParent.cpp#1872

Hmm, this doesn't seem quite right. I think the hooked window procedure would still get called here. I'm trying to think of cases in content where we have a native window that needs events processed for it. Nothing comes to mind.

Comment 9

[Jim Mathies [:jimm]]

I tried to land this same fix back in 2016 and bill felt we should keep it (bug 1160140). I can't come up with a case where this deferral behavior is needed in the content process. The example bill gave doesn't get addressed by deferring in content.

plugin (sync) -> content (sync) -> chrome (sendmessage) -> plugin : known deadlock case
plugin (sync) -> chrome (sync) -> content (sendmessage or sync) -> plugin: handled by child side deferral
chrome -> content -> plugin (sendmessage) -> chrome : handled by chrome side deferral

since we don't have windows in content, I don't think we need this.

Comment 10

[Jim Mathies [:jimm]]

I've been thinking about ways this prevents deadlocks in ipc messaging and I'm not coming up with anything. I think I'll revamp this patch to just remove the deferred handling independent of the native events switch.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad2edcb85248
Turn off REQUIRE_DEFERRED_MESSAGE_PROTECTION in PluginModuleContentParent when native events are disabled. r=bobowen

Comment 12

[Cosmin Sabou [:CosminS]]

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