Investigate why PPluginWidget's RecvCreate is failure prone

RESOLVED FIXED in Firefox 38

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

Trunk
mozilla38
x86_64
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm5+, firefox38 fixed)

Details

Attachments

(1 attachment)

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.
Assignee: nobody → jmathies
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.
Posted patch patchSplinter Review
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.
Attachment #8559442 - Flags: review?(roc)
(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. :)
tracking-e10s: --- → m5+
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.
https://hg.mozilla.org/mozilla-central/rev/cadacf4bc878
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
This worked well, 99% of these aborts are now gone.
You need to log in before you can comment on or make changes to this bug.