Remove nonsfi token workaround.

Remove nonsfi token workaround.

For Non-SFI plugins, NaCl executables retrieved via the "fast path" are no
longer registered, and file tokens are no longer generated. This prevents
unnecessary work from happening in the browser, and prevents tokens from being
exposed to Non-SFI plugins.

An earlier change was attempted that prevented using the "fast path" in the
case of non-sfi plugins, but a performance regression was observed on ARC.

BUG=394130
Committed: https://crrev.com/67c6eac492d4b26809bfc097e5fc303af74e2c02
Cr-Commit-Position: refs/heads/master@{#311758}

[teravest, 2015-01-12 17:00:57]

Patchset #2 (id:20001) has been deleted

[teravest, 2015-01-12 17:03:07]

Patchset #1 (id:1) has been deleted

[teravest, 2015-01-12 17:03:41]

teravest@chromium.org changed reviewers:
+ mseaborn@chromium.org

[Mark Seaborn, 2015-01-12 21:59:35]

https://codereview.chromium.org/840103003/diff/40001/components/nacl/browser/...
File components/nacl/browser/nacl_file_host.cc (right):

https://codereview.chromium.org/840103003/diff/40001/components/nacl/browser/...
components/nacl/browser/nacl_file_host.cc:125: bool register_executable,
How about calling this something like "enable_validation_caching"?

Understanding "register_executable" requires knowing more context of how this
part of validation caching works.

https://codereview.chromium.org/840103003/diff/40001/components/nacl/browser/...
components/nacl/browser/nacl_file_host.cc:142: if (register_executable) {
Can you add a comment to explain that: Both branches of this conditional will
behave correctly but have different performance trade-offs.  One registers the
FD for use with validation caching.  Registering the FD carries a small up-front
penalty but saves time later -- but that penalty isn't wanted in Non-SFI Mode
which doesn't need validation caching.

Without the explanation, this code is rather obscure. :-)

[teravest, 2015-01-13 16:47:34]

https://codereview.chromium.org/840103003/diff/40001/components/nacl/browser/...
File components/nacl/browser/nacl_file_host.cc (right):

https://codereview.chromium.org/840103003/diff/40001/components/nacl/browser/...
components/nacl/browser/nacl_file_host.cc:125: bool register_executable,
On 2015/01/12 21:59:35, Mark Seaborn wrote:
> How about calling this something like "enable_validation_caching"?
> 
> Understanding "register_executable" requires knowing more context of how this
> part of validation caching works.

Done.

https://codereview.chromium.org/840103003/diff/40001/components/nacl/browser/...
components/nacl/browser/nacl_file_host.cc:142: if (register_executable) {
On 2015/01/12 21:59:34, Mark Seaborn wrote:
> Can you add a comment to explain that: Both branches of this conditional will
> behave correctly but have different performance trade-offs.  One registers the
> FD for use with validation caching.  Registering the FD carries a small
up-front
> penalty but saves time later -- but that penalty isn't wanted in Non-SFI Mode
> which doesn't need validation caching.
> 
> Without the explanation, this code is rather obscure. :-)

Good point. I've added a comment here.

[Mark Seaborn, 2015-01-15 00:15:07]

LGTM

[teravest, 2015-01-15 03:21:10]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2015-01-15 03:22:13]

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

[commit-bot: I haz the power, 2015-01-15 03:26:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-15 03:26:02]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[teravest, 2015-01-15 15:35:24]

teravest@chromium.org changed reviewers:
+ tsepez@chromium.org

[teravest, 2015-01-15 15:35:25]

+tsepez for components/nacl/common/nacl_host_messages.h

This adds a field to NaClHostMsg_OpenNaClExecutable which allows executables to
be opened on the browser without registering a token for later validation cache
use.

In the case of Non-SFI mode, it's not necessary to generate a token and send it
to the plugin; the plugin will never make any calls to the validation cache.

[Tom Sepez, 2015-01-15 17:16:28]

> In the case of Non-SFI mode, it's not necessary to generate a token and send
it
> to the plugin; the plugin will never make any calls to the validation cache.
And presumably, should it try, it will fail because there isn't a token.  Can we
cover that case with a test?

[teravest, 2015-01-15 20:53:00]

NaClProcessMsg_ResolveFileToken messages aren't even handled by the browser when
nonsfi mode is used:
https://code.google.com/p/chromium/codesearch#chromium/src/components/nacl/br...

...so there's no good way to cover that case with a test.

[Tom Sepez, 2015-01-15 21:27:11]

>
https://code.google.com/p/chromium/codesearch#chromium/src/components/nacl/br...
> 
> ...so there's no good way to cover that case with a test.
Fair enough. LGTM.

[teravest, 2015-01-15 21:33:13]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2015-01-15 21:38:11]

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

[commit-bot: I haz the power, 2015-01-15 23:09:24]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2015-01-15 23:10:46]

Patchset 2 (id:??) landed as
https://crrev.com/67c6eac492d4b26809bfc097e5fc303af74e2c02
Cr-Commit-Position: refs/heads/master@{#311758}