Enable PNaCl by default (revert disabling CL)

Enable PNaCl by default (revert disabling CL)

The first of the two security issues is resolved, and the second is ready.  To enable more developer exposure in dev channel.

Revert https://codereview.chromium.org/22839005 and remove two small pieces of related dead code from the plugin.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3633
R=jvoung@chromium.org, mseaborn@chromium.org, thakis@chromium.org

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

[sehr, 2013-08-20 18:37:12]

This is to enable pnacl by default for M31.  PTAL.

[Mark Seaborn, 2013-08-20 19:22:45]

Is this just a revert of a previous commit?  If so, please say so (and give the
revision number) in the commit message.

[sehr, 2013-08-20 19:57:03]

PTAL.

[jvoung (off chromium), 2013-08-20 20:32:52]

https://codereview.chromium.org/23326003/diff/1/chrome/common/chrome_switches.h
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/23326003/diff/1/chrome/common/chrome_switches...
chrome/common/chrome_switches.h:192: extern const char kDisablePnacl[];
nit: should sort

https://codereview.chromium.org/23326003/diff/1/chrome/test/nacl/nacl_browser...
File chrome/test/nacl/nacl_browsertest.cc (right):

https://codereview.chromium.org/23326003/diff/1/chrome/test/nacl/nacl_browser...
chrome/test/nacl/nacl_browsertest.cc:164:
IN_PROC_BROWSER_TEST_F(NaClBrowserTestPnacl, MAYBE_PNACL(PnaclMimeType)) {
Now that I think of it, you don't need the MAYBE_PNACL() here, since it's not
actually loading a pexe.

The MAYBE_PNACL() macro exists there because on windows unoptimized debug
builds, the validator and validation cache are too slow and cause timeouts.

https://codereview.chromium.org/23326003/diff/1/ppapi/native_client/src/trust...
File ppapi/native_client/src/trusted/plugin/service_runtime.cc (left):

https://codereview.chromium.org/23326003/diff/1/ppapi/native_client/src/trust...
ppapi/native_client/src/trusted/plugin/service_runtime.cc:357:
p->error_info->SetReport(ERROR_PNACL_NOT_ENABLED,
Comment that the UMA entry is no longer used in the enum, but comment that
people should not remove it so as to preserve the UMA numbers?

[sehr, 2013-08-20 21:36:39]

Thanks, Jan.  PTAL.

https://codereview.chromium.org/23326003/diff/1/chrome/common/chrome_switches.h
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/23326003/diff/1/chrome/common/chrome_switches...
chrome/common/chrome_switches.h:192: extern const char kDisablePnacl[];
On 2013/08/20 20:32:52, jvoung (cr) wrote:
> nit: should sort

Done.

https://codereview.chromium.org/23326003/diff/1/chrome/test/nacl/nacl_browser...
File chrome/test/nacl/nacl_browsertest.cc (right):

https://codereview.chromium.org/23326003/diff/1/chrome/test/nacl/nacl_browser...
chrome/test/nacl/nacl_browsertest.cc:164:
IN_PROC_BROWSER_TEST_F(NaClBrowserTestPnacl, MAYBE_PNACL(PnaclMimeType)) {
On 2013/08/20 20:32:52, jvoung (cr) wrote:
> Now that I think of it, you don't need the MAYBE_PNACL() here, since it's not
> actually loading a pexe.
> 
> The MAYBE_PNACL() macro exists there because on windows unoptimized debug
> builds, the validator and validation cache are too slow and cause timeouts.

Removed in both places.

https://codereview.chromium.org/23326003/diff/1/ppapi/native_client/src/trust...
File ppapi/native_client/src/trusted/plugin/service_runtime.cc (left):

https://codereview.chromium.org/23326003/diff/1/ppapi/native_client/src/trust...
ppapi/native_client/src/trusted/plugin/service_runtime.cc:357:
p->error_info->SetReport(ERROR_PNACL_NOT_ENABLED,
On 2013/08/20 20:32:52, jvoung (cr) wrote:
> Comment that the UMA entry is no longer used in the enum, but comment that
> people should not remove it so as to preserve the UMA numbers?

Done.

[jvoung (off chromium), 2013-08-20 21:57:08]

lgtm

[sehr, 2013-08-20 23:51:48]

On 2013/08/20 21:57:08, jvoung (cr) wrote:
> lgtm

Thanks, Jan.  I still need reviews of the chrome components from jhawkins, or a
suggestion for other reviewers.

David

[sehr, 2013-08-21 21:48:31]

I still need a review of the changes in src/chrome.  PTAL.

[Nico, 2013-08-21 21:59:27]

I don't know why nacl was disabled (the cl description of the disable cl didn't
say) and I don't know why it should be turned on again (neither this cl
description nor the bug it links to say why), so it sounds like I lack context
for this CL.

[sehr, 2013-08-22 18:49:04]

Nico,

Agreed I should have documented the reason for disabling and re-enabling.  I
have updated the relevant change list comments, I hope to your satisfaction. 
PTAL.

David

[Nico, 2013-08-22 18:51:11]

Thanks!

lgtm stamp

[Mark Seaborn, 2013-08-22 21:06:23]

LGTM for chrome/test/data/nacl.

[sehr, 2013-08-22 21:14:37]

Committed patchset #2 manually as r219104 (presubmit successful).