554171 - Add thread checks to several functions that must only be called on the main thread

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

_getvalueforurl
_setvalueforurl
_getauthenticationinfo
_scheduletimer
_unscheduletimer
_popupcontextmenu
_convertpoint

All these need thread checks.

Comment 1

[Chris Peterson [:cpeterson]]

Attachment: plugin-main-thread-checks.patch

1. Add plugin thread checks to functions that must only be called on the main thread.

I did some basic testing of Flash and Silverlight and didn't see them calling these APIs from a non-main thread, but they could still have some uncommon code paths that might. This bug has been open for five years and these APIs modify global shared state, so presumably plugins are well behaved or we have random crashes that we have just not attributed to these data races?

2. s/NPN_write/NPN_destroystream/ in _destroystream log message.

3. Remove redundant !url check in _setvalueforurl.

Comment 2

[Jim Mathies [:jimm]]

Comment on attachment 8648588 [details] [diff] [review]
plugin-main-thread-checks.patch

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

::: dom/plugins/base/nsNPAPIPlugin.cpp
@@ +2703,5 @@
>  
>    switch (variable) {
>    case NPNURLVCookie:
>      {
> +      if (!value || 0 == len)

lets reverse this - len == 0, or since this is unsigned, I think you could just do !len.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/08227e3e08b8

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/08227e3e08b8