Instrument pepper plugin load failures.

Instrument pepper plugin load failures.

ChromeOS experiences an increased failure to load Pepper Flash plugins.
To help investigate these failures in the field increase logging.
Also disallow npapi plugin generation on ChromeOS as a fallback. We
don't need a zombie process when this failure happens.

BUG=chromium:314301
TEST=kill -stop, reload, kill -9 ppapi process on Pixel.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=239740

[ilja, 2013-12-07 02:51:35]

I tried finding possible points of failure and added spew to them. I couldn't
reproduce most spew on Pixel with the kills. Please let me know if you think
there might be other places. For instance I am wondering if process_->Launch
will always lead to OnProcessLaunched or if that could fail somehow.

[ilja, 2013-12-07 03:00:45]

On 2013/12/07 02:51:35, ilja wrote:
> I tried finding possible points of failure and added spew to them. I couldn't
> reproduce most spew on Pixel with the kills. Please let me know if you think
> there might be other places. For instance I am wondering if process_->Launch
> will always lead to OnProcessLaunched or if that could fail somehow.

Actually I just tried skipping that once and it gave me a channel error from
which it recovered.

[piman, 2013-12-09 22:39:06]

Can we make those VLOG? We can then add logging only on Chrome OS (or when doing
debugging) by using the --vmodule there, to avoid spew on all other platforms.

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
File content/browser/plugin_service_impl.cc (right):

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
content/browser/plugin_service_impl.cc:336: LOG(WARNING) << "Unable to find
ppapi plugin registration."
nit: space after .

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
content/browser/plugin_service_impl.cc:353: if (!plugin_host)
nit: needs braces per style.

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
content/browser/plugin_service_impl.cc:354: LOG(WARNING) << "Unable to create
ppapi plugin process."
nit: space after .

https://codereview.chromium.org/103623003/diff/20001/content/browser/ppapi_pl...
File content/browser/ppapi_plugin_process_host.cc (right):

https://codereview.chromium.org/103623003/diff/20001/content/browser/ppapi_pl...
content/browser/ppapi_plugin_process_host.cc:120: CHECK(plugin_host);
nit: DCHECK

[Scott Hess - ex-Googler, 2013-12-09 23:47:45]

I'm not sure I understand this code well enough to do a great review, without
spending time recovering lost knowledge.  Let me know if you feel that is
necessary.

Would it be worthwhile to instrument with histograms, so that you don't have to
rely on users to deliver things?

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
File content/browser/plugin_service_impl.cc (right):

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
content/browser/plugin_service_impl.cc:336: LOG(WARNING) << "Unable to find
ppapi plugin registration."
On 2013/12/09 22:39:06, piman wrote:
> nit: space after .

Or rewrite "Unable to ... for: " << path.  And below.

https://codereview.chromium.org/103623003/diff/20001/content/browser/ppapi_pl...
File content/browser/ppapi_plugin_process_host.cc (right):

https://codereview.chromium.org/103623003/diff/20001/content/browser/ppapi_pl...
content/browser/ppapi_plugin_process_host.cc:120: CHECK(plugin_host);
On 2013/12/09 22:39:06, piman wrote:
> nit: DCHECK

Wouldn't NULL be obvious from the crash reports anyhow?

[ilja, 2013-12-10 05:55:31]

Antoine mentioned the zygote and I decided to promote debug warnings to release
for plugin load failures that could happen early there.

PTAL.

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
File content/browser/plugin_service_impl.cc (right):

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
content/browser/plugin_service_impl.cc:336: LOG(WARNING) << "Unable to find
ppapi plugin registration."
On 2013/12/09 22:39:06, piman wrote:
> nit: space after .

Done.

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
content/browser/plugin_service_impl.cc:336: LOG(WARNING) << "Unable to find
ppapi plugin registration."
On 2013/12/09 23:47:45, shess wrote:
> On 2013/12/09 22:39:06, piman wrote:
> > nit: space after .
> 
> Or rewrite "Unable to ... for: " << path.  And below.

Done.

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
content/browser/plugin_service_impl.cc:353: if (!plugin_host)
On 2013/12/09 22:39:06, piman wrote:
> nit: needs braces per style.

Done.

https://codereview.chromium.org/103623003/diff/20001/content/browser/plugin_s...
content/browser/plugin_service_impl.cc:354: LOG(WARNING) << "Unable to create
ppapi plugin process."
On 2013/12/09 22:39:06, piman wrote:
> nit: space after .

Done.

https://codereview.chromium.org/103623003/diff/20001/content/browser/ppapi_pl...
File content/browser/ppapi_plugin_process_host.cc (right):

https://codereview.chromium.org/103623003/diff/20001/content/browser/ppapi_pl...
content/browser/ppapi_plugin_process_host.cc:120: CHECK(plugin_host);
On 2013/12/09 22:39:06, piman wrote:
> nit: DCHECK

Done.

https://codereview.chromium.org/103623003/diff/20001/content/browser/ppapi_pl...
content/browser/ppapi_plugin_process_host.cc:120: CHECK(plugin_host);
On 2013/12/09 23:47:45, shess wrote:
> On 2013/12/09 22:39:06, piman wrote:
> > nit: DCHECK
> 
> Wouldn't NULL be obvious from the crash reports anyhow?

It has virtuals, so it should crash when accessing it in the next line. I guess
it isn't really needed, but I am keeping it a DCHECK just in case.

[ilja, 2013-12-10 06:50:24]

Changed logs to VLOG and added patterns to session_manager_setup.sh
https://chromium-review.googlesource.com/#/c/179421/1/session_manager_setup.sh

[piman, 2013-12-10 06:58:37]

lgtm

[commit-bot: I haz the power, 2013-12-10 06:59:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ihf@chromium.org/103623003/100001

[commit-bot: I haz the power, 2013-12-10 09:14:54]

Change committed as 239740