Fix a bug that would cause the plugin_loader to stall.

Fix a bug that would cause the plugin_loader to stall.

If there's an error launching the plugin_loader's process_host, then the
plugin_loader will stall forever. This happened because the return value of
process_host_->Send() was being ignored, and because PluginLoaderPosix was not
overriding OnProcessLaunchFailed().

This CL also includes a minor refactor:
- Removed the LOCAL_HISTOGRAM_TIMES "PluginLoaderPosix.LoadDone", because it
was measuring the time between MaybeRunPendingCallbacks(), rather than the
amount of time it takes to load the plugins. I'm guessing that this metric is
out of date, and was never removed.
- Separated MaybeRunPendingCallbacks() into two methods,
IsFinishedLoadingPlugins() and FinishedLoadingPlugins().
- Removed the method MaybeAddInternalPlugin(), as its name was confusing, and
added FindInternalPlugin() in its place. This also simplified the logic
slightly.

BUG=429388
Committed: https://crrev.com/f610f1386915840bee7de38377b5e5b80f6f076e
Cr-Commit-Position: refs/heads/master@{#303581}

[erikchen, 2014-11-11 00:33:26]

erikchen@chromium.org changed reviewers:
+ avi@chromium.org

[erikchen, 2014-11-11 00:33:26]

avi: Please review.

[erikchen, 2014-11-11 00:48:13]

Patchset #1 (id:1) has been deleted

[erikchen, 2014-11-11 00:48:18]

Patchset #1 (id:20001) has been deleted

[Avi (use Gerrit), 2014-11-11 02:38:06]

lgtm

https://codereview.chromium.org/718523002/diff/80001/content/browser/plugin_l...
File content/browser/plugin_loader_posix.cc (right):

https://codereview.chromium.org/718523002/diff/80001/content/browser/plugin_l...
content/browser/plugin_loader_posix.cc:42: // SetPlugins()
nit: end comments with a .

I don't quite follow the comment. So PrepareForPluginLoading() ends up calling
SetPlugins? (I'm not that familiar with plugin code.)

https://codereview.chromium.org/718523002/diff/80001/content/browser/plugin_l...
content/browser/plugin_loader_posix.cc:174: std::vector<WebPluginInfo>::iterator
it = FindInternalPlugin(plugin.path);
auto it = ...?

https://codereview.chromium.org/718523002/diff/80001/content/browser/plugin_l...
content/browser/plugin_loader_posix.cc:198: std::vector<WebPluginInfo>::iterator
it = FindInternalPlugin(plugin_path);
auto it = ...?

https://codereview.chromium.org/718523002/diff/80001/content/browser/plugin_l...
content/browser/plugin_loader_posix.cc:212: return plugin.path == plugin_path;
Yay lambdas!

[erikchen, 2014-11-11 02:44:48]

https://codereview.chromium.org/718523002/diff/80001/content/browser/plugin_l...
File content/browser/plugin_loader_posix.cc (right):

https://codereview.chromium.org/718523002/diff/80001/content/browser/plugin_l...
content/browser/plugin_loader_posix.cc:42: // SetPlugins()
On 2014/11/11 02:38:06, Avi wrote:
> nit: end comments with a .
> 
> I don't quite follow the comment. So PrepareForPluginLoading() ends up calling
> SetPlugins? (I'm not that familiar with plugin code.)

I changed the comment to be more clear. "When |loading_plugins_| is set to
false, this instance must call SetPlugins()."

No behavioral change, just adding a much needed comment.
PluginList::Singleton()->PrepareForPluginLoading(); requires an eventual
matching call to SetPlugins(). This class does this, but doesn't explicitly
state this.

https://codereview.chromium.org/718523002/diff/80001/content/browser/plugin_l...
content/browser/plugin_loader_posix.cc:174: std::vector<WebPluginInfo>::iterator
it = FindInternalPlugin(plugin.path);
On 2014/11/11 02:38:06, Avi wrote:
> auto it = ...?

Oh yes, delightful.

https://codereview.chromium.org/718523002/diff/80001/content/browser/plugin_l...
content/browser/plugin_loader_posix.cc:198: std::vector<WebPluginInfo>::iterator
it = FindInternalPlugin(plugin_path);
On 2014/11/11 02:38:06, Avi wrote:
> auto it = ...?

Done.

https://codereview.chromium.org/718523002/diff/80001/content/browser/plugin_l...
content/browser/plugin_loader_posix.cc:212: return plugin.path == plugin_path;
On 2014/11/11 02:38:06, Avi wrote:
> Yay lambdas!

\o/

[erikchen, 2014-11-11 02:44:53]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-11-11 02:46:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/718523002/100001

[commit-bot: I haz the power, 2014-11-11 03:28:26]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2014-11-11 03:29:57]

Patchset 4 (id:??) landed as
https://crrev.com/f610f1386915840bee7de38377b5e5b80f6f076e
Cr-Commit-Position: refs/heads/master@{#303581}