Trusted plugin: Clean up manifest error reporting.

This streamlines some of the error reporting in the Manifest support classes in
the trusted plugin. Without this change, it's a bit hard to tell when we report
errors and when we don't; this makes some logic more explicit for when errors
are reported.

This will hopefully make reviewing the change that moves the manifest support
out of the trusted plugin easier.

BUG=353592

Review URL: https://codereview.chromium.org/266723004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267624 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 31 insertions, 50 deletions

diff --git a/ppapi/native_client/src/trusted/plugin/json_manifest.cc b/ppapi/native_client/src/trusted/plugin/json_manifest.cc
index 0187497..418339c 100644
--- a/ppapi/native_client/src/trusted/plugin/json_manifest.cc
+++ b/ppapi/native_client/src/trusted/plugin/json_manifest.cc
@@ -515,8 +515,9 @@ bool JsonManifest::GetURLFromISADictionary(const Json::Value& dictionary,
 
   // When the application actually requests a resolved URL, we must have
   // a matching entry (sandbox_isa_ or portable) for NaCl.
+  ErrorInfo ignored_error_info;
   if (!IsValidISADictionary(dictionary, parent_key, sandbox_isa_, true,
-                            nonsfi_enabled_, error_info)) {
+                            nonsfi_enabled_, &ignored_error_info)) {
     error_info->SetReport(PP_NACL_ERROR_MANIFEST_RESOLVE_URL,
                           "architecture " + sandbox_isa_ +
                           " is not found for file " + parent_key);
@@ -567,22 +568,25 @@ bool JsonManifest::GetURLFromISADictionary(const Json::Value& dictionary,
 bool JsonManifest::GetKeyUrl(const Json::Value& dictionary,
                              const nacl::string& key,
                              nacl::string* full_url,
-                             PP_PNaClOptions* pnacl_options,
-                             ErrorInfo* error_info) const {
-  DCHECK(full_url != NULL && pnacl_options != NULL && error_info != NULL);
+                             PP_PNaClOptions* pnacl_options) const {
+  DCHECK(full_url != NULL && pnacl_options != NULL);
   if (!dictionary.isMember(key)) {
-    error_info->SetReport(PP_NACL_ERROR_MANIFEST_RESOLVE_URL,
-                          "file key not found in manifest");
+    PLUGIN_PRINTF(("file key not found in manifest"));
     return false;
   }
   const Json::Value& isa_dict = dictionary[key];
   nacl::string relative_url;
   bool uses_nonsfi_mode;
+
+  // We ignore the error_info we receive here but it's needed for the calls
+  // below.
+  ErrorInfo error_info;
+
   if (!GetURLFromISADictionary(isa_dict, key, &relative_url,
-                               pnacl_options, &uses_nonsfi_mode, error_info)) {
+                               pnacl_options, &uses_nonsfi_mode, &error_info)) {
     return false;
   }
-  return ResolveURL(relative_url, full_url, error_info);
+  return ResolveURL(relative_url, full_url, &error_info);
 }
 
 bool JsonManifest::GetProgramURL(nacl::string* full_url,
@@ -593,10 +597,7 @@ bool JsonManifest::GetProgramURL(nacl::string* full_url,
     return false;
 
   const Json::Value& program = dictionary_[kProgramKey];
-
   nacl::string nexe_url;
-  nacl::string error_string;
-
   if (!GetURLFromISADictionary(program,
                                kProgramKey,
                                &nexe_url,
@@ -605,37 +606,32 @@ bool JsonManifest::GetProgramURL(nacl::string* full_url,
                                error_info)) {
     return false;
   }
-
   return ResolveURL(nexe_url, full_url, error_info);
 }
 
 bool JsonManifest::ResolveKey(const nacl::string& key,
                               nacl::string* full_url,
-                              PP_PNaClOptions* pnacl_options,
-                              ErrorInfo* error_info) const {
+                              PP_PNaClOptions* pnacl_options) const {
   NaClLog(3, "JsonManifest::ResolveKey(%s)\n", key.c_str());
   // key must be one of kProgramKey or kFileKey '/' file-section-key
 
-  if (full_url == NULL || pnacl_options == NULL || error_info == NULL)
+  if (full_url == NULL || pnacl_options == NULL)
     return false;
 
-  if (key == kProgramKey) {
-    return GetKeyUrl(dictionary_, key, full_url, pnacl_options, error_info);
-  }
+  if (key == kProgramKey)
+    return GetKeyUrl(dictionary_, key, full_url, pnacl_options);
   nacl::string::const_iterator p = find(key.begin(), key.end(), '/');
   if (p == key.end()) {
-    error_info->SetReport(PP_NACL_ERROR_MANIFEST_RESOLVE_URL,
-                          nacl::string("ResolveKey: invalid key, no slash: ")
-                          + key);
+    std::string err = "ResolveKey: invalid key, no slash: " + key;
+    PLUGIN_PRINTF((err.c_str()));
     return false;
   }
 
   // generalize to permit other sections?
   nacl::string prefix(key.begin(), p);
   if (prefix != kFilesKey) {
-    error_info->SetReport(PP_NACL_ERROR_MANIFEST_RESOLVE_URL,
-                          nacl::string("ResolveKey: invalid key: not \"files\""
-                                       " prefix: ") + key);
+    std::string err = "ResolveKey: invalid key: no \"files\" prefix: " + key;
+    PLUGIN_PRINTF((err.c_str()));
     return false;
   }
 
@@ -643,18 +639,16 @@ bool JsonManifest::ResolveKey(const nacl::string& key,
 
   const Json::Value& files = dictionary_[kFilesKey];
   if (!files.isObject()) {
-    error_info->SetReport(
-        PP_NACL_ERROR_MANIFEST_RESOLVE_URL,
-        nacl::string("ResolveKey: no \"files\" dictionary"));
+    std::string err = "ResolveKey: no \"files\" dictionary";
+    PLUGIN_PRINTF((err.c_str()));
     return false;
   }
   if (!files.isMember(rest)) {
-    error_info->SetReport(
-        PP_NACL_ERROR_MANIFEST_RESOLVE_URL,
-        nacl::string("ResolveKey: no such \"files\" entry: ") + key);
+    std::string err = "ResolveKey: no such \"files\" entry: " + key;
+    PLUGIN_PRINTF((err.c_str()));
     return false;
   }
-  return GetKeyUrl(files, rest, full_url, pnacl_options, error_info);
+  return GetKeyUrl(files, rest, full_url, pnacl_options);
 }
 
 bool JsonManifest::ResolveURL(const nacl::string& relative_url,
diff --git a/ppapi/native_client/src/trusted/plugin/json_manifest.h b/ppapi/native_client/src/trusted/plugin/json_manifest.h
index dd7e3ab..cf02798 100644
--- a/ppapi/native_client/src/trusted/plugin/json_manifest.h
+++ b/ppapi/native_client/src/trusted/plugin/json_manifest.h
@@ -57,11 +57,9 @@ class JsonManifest : public Manifest {
   // Resolves a key from the "files" section to a fully resolved URL,
   // i.e., relative URL values are fully expanded relative to the
   // manifest's URL (via ResolveURL).
-  // If there was an error, details are reported via error_info.
   virtual bool ResolveKey(const nacl::string& key,
                           nacl::string* full_url,
-                          PP_PNaClOptions* pnacl_options,
-                          ErrorInfo* error_info) const;
+                          PP_PNaClOptions* pnacl_options) const;
 
  private:
   NACL_DISALLOW_COPY_AND_ASSIGN(JsonManifest);
@@ -79,8 +77,7 @@ class JsonManifest : public Manifest {
   bool GetKeyUrl(const Json::Value& dictionary,
                  const nacl::string& key,
                  nacl::string* full_url,
-                 PP_PNaClOptions* pnacl_options,
-                 ErrorInfo* error_info) const;
+                 PP_PNaClOptions* pnacl_options) const;
 
   bool GetURLFromISADictionary(const Json::Value& dictionary,
                                const nacl::string& parent_key,
diff --git a/ppapi/native_client/src/trusted/plugin/manifest.h b/ppapi/native_client/src/trusted/plugin/manifest.h
index 03ed71e..6a74dd6 100644
--- a/ppapi/native_client/src/trusted/plugin/manifest.h
+++ b/ppapi/native_client/src/trusted/plugin/manifest.h
@@ -52,11 +52,9 @@ class Manifest {
   // manifest's URL.  Fills in |pnacl_options| if
   // the resolved key requires a pnacl translation step to obtain
   // the final requested resource.
-  // If there was an error, details are reported via error_info.
   virtual bool ResolveKey(const nacl::string& key,
                           nacl::string* full_url,
-                          PP_PNaClOptions* pnacl_options,
-                          ErrorInfo* error_info) const = 0;
+                          PP_PNaClOptions* pnacl_options) const = 0;
 
  protected:
   NACL_DISALLOW_COPY_AND_ASSIGN(Manifest);
diff --git a/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc b/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc
index fd108b7..93d2220 100644
--- a/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc
+++ b/ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc
@@ -58,16 +58,14 @@ class PnaclManifest : public Manifest {
 
   virtual bool ResolveKey(const nacl::string& key,
                           nacl::string* full_url,
-                          PP_PNaClOptions* pnacl_options,
-                          ErrorInfo* error_info) const {
+                          PP_PNaClOptions* pnacl_options) const {
     // All of the component files are native (do not require pnacl translate).
     pnacl_options->translate = PP_FALSE;
     // We can only resolve keys in the files/ namespace.
     const nacl::string kFilesPrefix = "files/";
     size_t files_prefix_pos = key.find(kFilesPrefix);
     if (files_prefix_pos == nacl::string::npos) {
-      error_info->SetReport(PP_NACL_ERROR_MANIFEST_RESOLVE_URL,
-                            "key did not start with files/");
+      PLUGIN_PRINTF(("key did not start with files/"));
       return false;
     }
     // Resolve the full URL to the file. Provide it with a platform-specific
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
index 131f9ae..7406e79 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
@@ -261,14 +261,8 @@ void PluginReverseInterface::OpenManifestEntry_MainThreadContinuation(
 
   std::string mapped_url;
   PP_PNaClOptions pnacl_options = {PP_FALSE, PP_FALSE, 2};
-  ErrorInfo error_info;
-  if (!manifest_->ResolveKey(p->url, &mapped_url,
-                             &pnacl_options, &error_info)) {
+  if (!manifest_->ResolveKey(p->url, &mapped_url, &pnacl_options)) {
     NaClLog(4, "OpenManifestEntry_MainThreadContinuation: ResolveKey failed\n");
-    NaClLog(4,
-            "Error code %d, string %s\n",
-            error_info.error_code(),
-            error_info.message().c_str());
     // Failed, and error_info has the details on what happened.  Wake
     // up requesting thread -- we are done.
     nacl::MutexLocker take(&mu_);