1129575 - Investigate why PPluginWidget's RecvCreate is failure prone

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Jim Mathies [:jimm]]

This call fails a lot, but it shows up as a failure in SendGetNativePluginPort, which is the first sync call we make after Create is sent over async.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I remember seeing this when I was testing the phoronix page. The STR was roughly as follows:

1. Create a bookmarks folder that contains a bunch of phoronix articles (maybe 20 or 30).
2. Select "Open All in Tabs" for that folder.
3. (Not sure about this) Quickly close the tabs while they're still loading.

Most of the time I would get a hang (which should not be fixed), but sometimes I think I got this failure.

Comment 2

[Jim Mathies [:jimm]]

Attachment: patch

Comment 3

[Jim Mathies [:jimm]]

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

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 8559442 [details] [diff] [review]
patch

This call is failing at a rather high rate. I'm guessing the cause is a bad parent, which we retrieve from TabParent right before we try to create the window. So I'm relaxing this failure case. The other two potential failures (oom, and a failure in native create window calls) still abort. I've also switched this back to sync so we have the return result in the content process.

Comment 5

[Jim Mathies [:jimm]]

(In reply to Bill McCloskey (:billm) from comment #1)
> I remember seeing this when I was testing the phoronix page. The STR was
> roughly as follows:
> 
> 1. Create a bookmarks folder that contains a bunch of phoronix articles
> (maybe 20 or 30).
> 2. Select "Open All in Tabs" for that folder.
> 3. (Not sure about this) Quickly close the tabs while they're still loading.
> 
> Most of the time I would get a hang (which should not be fixed), but
> sometimes I think I got this failure.

I've been testing this on hulu front page clip links, which sounds similar. I've reproduced the abort once in nightly there, but haven't seen it with these changes. Crash stats will be the ultimate judge though. :)

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 8559442 [details] [diff] [review]
patch

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

::: dom/ipc/TabChild.cpp
@@ +3487,5 @@
>      NS_WARNING("Creating native plugin widget on the chrome side failed.");
>    }
> +  *aOut = pluginWidget.get();
> +  pluginWidget.forget();
> +  return rv;

This code can be shortened to:

pluginWidget.forget(aOut);
return rv;

and it avoids an assert in debug builds.

Comment 7

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cadacf4bc878

Thanks!

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Jim Mathies [:jimm]]

This worked well, 99% of these aborts are now gone.