Set file tokens for NaCl main nexe if available to enable validation caching.

ppapi/native_client/src/trusted/plugin/plugin.cc

Index: ppapi/native_client/src/trusted/plugin/plugin.cc
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index c2fcbd87b07801c03ebfeb60554b8d3129bbd354..dcba9c56454ec012486df222f7bfbd5ffde69bfa 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -25,6 +25,7 @@
 #include "native_client/src/include/portability_string.h"
 #include "native_client/src/shared/platform/nacl_check.h"
 #include "native_client/src/trusted/desc/nacl_desc_wrapper.h"
+#include "native_client/src/trusted/desc_cacheability/desc_cacheability.h"
 #include "native_client/src/trusted/nonnacl_util/sel_ldr_launcher.h"
 #include "native_client/src/trusted/service_runtime/nacl_error_code.h"
 
@@ -488,8 +489,19 @@ void Plugin::NexeFileDidOpen(int32_t pp_error) {
   if (nexe_bytes_read == -1)
     return;
 
+  // Hand desc ownership over to the wrapper. Keep the tokens in tmp_info
+  // before losing the tokens.
+  tmp_info = info.Release();
   nacl::scoped_ptr<nacl::DescWrapper>
-      wrapper(wrapper_factory()->MakeFileDesc(info.Release().desc, O_RDONLY));
+      wrapper(wrapper_factory()->MakeFileDesc(tmp_info.desc, O_RDONLY));
+  tmp_info.desc = -1;
+  if (tmp_info.file_token.lo != 0 && tmp_info.file_token.hi != 0) {

[teravest, 2014/04/30 20:12:54]

Should this be ||, not &&? Is it possible that lo or hi could be 0 for valid
token data?

[jvoung (off chromium), 2014/04/30 21:49:02]

Yeah I think having only one of them be 0 is still accepted.  I guess the
concern is if some code accidentally left one of the hi/lo at 0 all the time
(instead of just picking 0 by chance):

https://chromium.googlesource.com/native_client/src/native_client/+/refs/head...

And see NaClBrowser::PutFilePath which marches forth once of the tokens is
randomly chosen to be non-zero.

https://chromium.googlesource.com/chromium/chromium/+/trunk/components/nacl/b...

[teravest, 2014/05/02 15:07:25]

I'm confused. If you want to use && here, you should probably change
NaClBrowser::PutFilePath to match.

[jvoung (off chromium), 2014/05/02 15:33:23]

Oops sorry, messed up the negation logic. Changed to || to match.

[Nick Bray (chromium), 2014/05/02 18:18:19]

Justin is correct, although I'd go further and suggest this for clarity:
!(file_token->lo == 0 && file_token->hi == 0) .

See:
https://code.google.com/p/chromium/codesearch#chromium/src/native_client/src/...

Maybe this should even be factored into its own function, given a name, and
shared?  Unclear if that would be a win.

[jvoung (off chromium), 2014/05/02 19:26:15]

Yeah, I could put something in nacl_file_info.h, so that what's considered valid
is not just a comment in that file but a function that could be reused.

Then I can clean this up after a NaCl -> Chrome roll.

This particular check could also be folded into the desc_wrapper factory method
proposed in the other CL if that makes sense.

+    if (!NaClDescSetFileToken(wrapper->desc(), &tmp_info.file_token)) {

[teravest, 2014/04/30 20:12:54]

It's a shame that there's not a call to MakeFileDesc() that can take a
NaClFileInfo instead.

[jvoung (off chromium), 2014/04/30 21:49:02]

Okay, maybe we can add that:
https://codereview.chromium.org/261683002/

Thoughts, Nick?

[jvoung (off chromium), 2014/05/20 16:12:42]

Changed to use the new ctor function that passes both the desc and file token.

+      NaClLog(4, "NexeFileDidOpen: NaClDescSetFileToken failed\n");
+    }
+  } else {
+    NaClLog(4, "NexeFileDidOpen: No file tokens for NaCl main nexe\n");
+  }
   NaClLog(4, "NexeFileDidOpen: invoking LoadNaClModule\n");
   LoadNaClModule(
       wrapper.release(),