Closed Bug 1179972 Opened 5 years ago Closed 5 years ago

Sandbox related set parent calls for plugin windows can cause plugin hangs

Categories

(Core :: Plug-ins, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox41 --- fixed
firefox42 --- fixed

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(1 file)

Ran into this working on a bug today. Patch forthcoming.
Attached patch patchSplinter Review
Assignee: nobody → jmathies
Comment on attachment 8629079 [details] [diff] [review]
patch

Aside from some code cleanup, the only signifigant change here is the ReplyMessage added for WM_CHILDACTIVATE, which is the event I caught the hang on. The generator of that was the set parent call we added to fix a sandboxing issue. The child was caught up in a call to NPP_SetWindow.
Attachment #8629079 - Flags: review?(aklotz)
Attachment #8629079 - Flags: review?(aklotz) → review+
Thanks for fixing this Jim.

Just for my own education ...
How does PluginWidgetParent::SetParent interact with the Windows SetParent function that I moved in my patch?

Why does the cast in PluginWidgetParent::RecvSetNativeChildWindow need to be a C-style cast and not a C++ one?
Flags: needinfo?(jmathies)
(In reply to Bob Owen (:bobowen) from comment #3)
> Thanks for fixing this Jim.
> 
> Just for my own education ...
> How does PluginWidgetParent::SetParent interact with the Windows SetParent
> function that I moved in my patch?

It calls the native apis via the widget call to SetParent.

> Why does the cast in PluginWidgetParent::RecvSetNativeChildWindow need to be
> a C-style cast and not a C++ one?

This didn't build with vs 2013 when I turned the logging on.
Flags: needinfo?(jmathies)
https://hg.mozilla.org/mozilla-central/rev/bade3246f490
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
We should get this uplifted to Aurora, as the original patch is there at the moment.
Blocks: 1123759
Flags: needinfo?(jmathies)
Comment on attachment 8629079 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]:
bug 1123759
[User impact if declined]:
hangs
[Describe test coverage new/current, TreeHerder]:
landed on mc, baked for a while.
[Risks and why]:
low I think, fix for corner case hang problem.
[String/UUID change made/needed]:
na
Flags: needinfo?(jmathies)
Attachment #8629079 - Flags: approval-mozilla-aurora?
Setting status flags for Aurora.
Comment on attachment 8629079 [details] [diff] [review]
patch

Low risk, has baked on m-c for several days.
Attachment #8629079 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Blocks: 1185532
You need to log in before you can comment on or make changes to this bug.