Stop adding the "files/" prefix when sending open_resource IPC

components/nacl/renderer/json_manifest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/nacl/renderer/json_manifest.h"
 
 #include <set>
 
 #include "base/logging.h"
 #include "base/macros.h"
@@ skipping 445 matching lines @@
     if (GetKeyUrl(files, keys[i], &full_url, &unused_pnacl_options)) {
       if (GURL(full_url).SchemeIs("chrome-extension"))
         out_files->push_back(std::make_pair(keys[i], full_url));
     }
   }
 }
 
 bool JsonManifest::ResolveKey(const std::string& key,
                               std::string* full_url,
                               PP_PNaClOptions* pnacl_options) const {
   // key must be one of kProgramKey or kFileKey '/' file-section-key

[Mark Seaborn, 2015/04/16 18:47:47]

Update this comment (see my comment below)

[Yusuke Sato, 2015/04/16 20:37:20]

Done. I guess we can remove the comment.

   if (full_url == NULL || pnacl_options == NULL)
     return false;
 
   if (key == kProgramKey)

[Mark Seaborn, 2015/04/16 18:47:47]

Doesn't this case need to be removed now?

Otherwise, if you have a manifest file like this:

{
  "program": { "x86-32": ..., ... },
  "files": {
    "program": { "x86-32": ..., ... }
  }
}

then open_resource("program") will wrongly return the first entry instead of the
second.

AFAICT, before your change, no-one passes "program" to ResolveKey() (the code
uses GetProgramURL() instead), so it's safe to remove this case.

[Yusuke Sato, 2015/04/16 20:37:20]

Done.

     return GetKeyUrl(dictionary_, key, full_url, pnacl_options);
 
-  std::string::const_iterator p = std::find(key.begin(), key.end(), '/');
-  if (p == key.end()) {
-    VLOG(1) << "ResolveKey failed: invalid key, no slash: " << key;
-    return false;
-  }
-
   // generalize to permit other sections?

[Mark Seaborn, 2015/04/16 18:47:47]

You can remove this since comment it's clear we don't need to generalise this.

[Yusuke Sato, 2015/04/16 20:37:20]

Done.

-  std::string prefix(key.begin(), p);
-  if (prefix != kFilesKey) {
-    VLOG(1) << "ResolveKey failed: invalid key, no \"files\" prefix: " << key;
-    return false;
-  }
-
   const Json::Value& files = dictionary_[kFilesKey];
   if (!files.isObject()) {
     VLOG(1) << "ResolveKey failed: no \"files\" dictionary";
     return false;
   }
 
-  std::string rest(p + 1, key.end());
-  if (!files.isMember(rest)) {
+  if (!files.isMember(key)) {
     VLOG(1) << "ResolveKey failed: no such \"files\" entry: " << key;
     return false;
   }
-  return GetKeyUrl(files, rest, full_url, pnacl_options);
+  return GetKeyUrl(files, key, full_url, pnacl_options);
 }
 
 bool JsonManifest::MatchesSchema(ErrorInfo* error_info) {
   if (!dictionary_.isObject()) {
     error_info->error = PP_NACL_ERROR_MANIFEST_SCHEMA_VALIDATE;
     error_info->string = "manifest: is not a json dictionary.";
     return false;
   }
   Json::Value::Members members = dictionary_.getMemberNames();
   for (size_t i = 0; i < members.size(); ++i) {
@@ skipping 155 matching lines @@
   } else {
     // NaCl
     *url = isa_spec[kUrlKey].asString();
     pnacl_options->translate = PP_FALSE;
   }
 
   return true;
 }
 
 }  // namespace nacl