Closed Bug 1543173 Opened 5 months ago Closed 5 months ago

Turn off REQUIRE_DEFERRED_MESSAGE_PROTECTION in PluginModuleContentParent when native events are disabled

Categories

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

task

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: jimm, Assigned: jimm)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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

Assignee: nobody → jmathies
Priority: -- → P2
Attachment #9057233 - Attachment is obsolete: true

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

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

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

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.

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.

Pushed by jmathies@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ad2edcb85248
Turn off REQUIRE_DEFERRED_MESSAGE_PROTECTION in PluginModuleContentParent when native events are disabled. r=bobowen
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.