Pepper: Cleanup PNaCl support in trusted plugin.

This change cleans up some code in pnacl_coordinator and pnacl_resources to
remove code that can't be reached and simplify error reporting paths. This is
in preparation for moving all of this code out of the trusted plugin.

BUG=239656

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

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

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

@@ -264,7 +264,9 @@ nacl::DescWrapper* PnaclCoordinator::ReleaseTranslatedFD() {
 
 void PnaclCoordinator::ReportNonPpapiError(PP_NaClError err_code,
                                            const nacl::string& message) {
-  error_info_.SetReport(err_code, message);
+  ErrorInfo error_info;
+  error_info.SetReport(err_code, message);
+  plugin_->ReportLoadError(error_info);
   ExitWithError();
 }
 
@@ -273,16 +275,14 @@ void PnaclCoordinator::ReportPpapiError(PP_NaClError err_code,
                                         const nacl::string& message) {
   nacl::stringstream ss;
   ss << "PnaclCoordinator: " << message << " (pp_error=" << pp_error << ").";
-  error_info_.SetReport(err_code, ss.str());
+  ErrorInfo error_info;
+  error_info.SetReport(err_code, ss.str());
+  plugin_->ReportLoadError(error_info);
   ExitWithError();
 }
 
 void PnaclCoordinator::ExitWithError() {
-  PLUGIN_PRINTF(("PnaclCoordinator::ExitWithError (error_code=%d, "
-                 "message='%s')\n",
-                 error_info_.error_code(),
-                 error_info_.message().c_str()));
-  plugin_->ReportLoadError(error_info_);
+  PLUGIN_PRINTF(("PnaclCoordinator::ExitWithError\n"));
   // Free all the intermediate callbacks we ever created.
   // Note: this doesn't *cancel* the callbacks from the factories attached
   // to the various helper classes (e.g., pnacl_resources). Thus, those
@@ -310,6 +310,7 @@ void PnaclCoordinator::TranslateFinished(int32_t pp_error) {
   // Bail out if there was an earlier error (e.g., pexe load failure),
   // or if there is an error from the translation thread.
   if (translate_finish_error_ != PP_OK || pp_error != PP_OK) {
+    plugin_->ReportLoadError(error_info_);
     ExitWithError();
     return;
   }
@@ -581,9 +582,8 @@ void PnaclCoordinator::BitcodeStreamGotData(int32_t pp_error,
 
   translate_thread_->PutBytes(data, pp_error);
   // If pp_error > 0, then it represents the number of bytes received.
-  if (data && pp_error > 0) {
+  if (data && pp_error > 0)
     pexe_size_ += pp_error;
-  }
 }
 
 StreamCallback PnaclCoordinator::GetCallback() {

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

@@ -19,7 +19,6 @@
 namespace plugin {
 
 static const char kPnaclBaseUrl[] = "chrome://pnacl-translator/";
-const char PnaclUrls::kResourceInfoUrl[] = "pnacl.json";
 
 nacl::string PnaclUrls::GetBaseUrl() {
   return nacl::string(kPnaclBaseUrl);
@@ -51,6 +50,10 @@ nacl::string PnaclUrls::PnaclComponentURLToFilename(
   return r;
 }
 
+nacl::string PnaclUrls::GetResourceInfoUrl() {
+  return "pnacl.json";
+}
+
 //////////////////////////////////////////////////////////////////////
 
 PnaclResources::~PnaclResources() {
@@ -108,7 +111,7 @@ void PnaclResources::ReadResourceInfo(
   if (fd < 0) {
     // File-open failed. Assume this means that the file is
     // not actually installed.
-    ReadResourceInfoError(
+    coordinator_->ReportNonPpapiError(PP_NACL_ERROR_PNACL_RESOURCE_FETCH,
         nacl::string("The Portable Native Client (pnacl) component is not "
                      "installed. Please consult chrome://components for more "
                      "information."));
@@ -118,7 +121,7 @@ void PnaclResources::ReadResourceInfo(
   nacl::string json_buffer;
   file_utils::StatusCode status = file_utils::SlurpFile(fd, json_buffer);
   if (status != file_utils::PLUGIN_FILE_SUCCESS) {
-    ReadResourceInfoError(
+    coordinator_->ReportNonPpapiError(PP_NACL_ERROR_PNACL_RESOURCE_FETCH,
         nacl::string("PnaclResources::ReadResourceInfo reading "
                      "failed for: ") + resource_info_filename);
     return;
@@ -126,63 +129,54 @@ void PnaclResources::ReadResourceInfo(
 
   // Finally, we have the resource info JSON data in json_buffer.
   PLUGIN_PRINTF(("Resource info JSON data:\n%s\n", json_buffer.c_str()));
-  nacl::string error_message;
-  if (!ParseResourceInfo(json_buffer, error_message)) {
-    ReadResourceInfoError(nacl::string("Parsing resource info failed: ") +
-                          error_message + "\n");
+  if (!ParseResourceInfo(json_buffer))
     return;
-  }
 
   // Done. Queue the completion callback.
   pp::Core* core = pp::Module::Get()->core();
   core->CallOnMainThread(0, resource_info_read_cb, PP_OK);
 }
 
-void PnaclResources::ReadResourceInfoError(const nacl::string& msg) {
-  coordinator_->ReportNonPpapiError(PP_NACL_ERROR_PNACL_RESOURCE_FETCH, msg);
-}
-
-bool PnaclResources::ParseResourceInfo(const nacl::string& buf,
-                                       nacl::string& errmsg) {
+bool PnaclResources::ParseResourceInfo(const nacl::string& buf) {
   // Expect the JSON file to contain a top-level object (dictionary).
   Json::Reader json_reader;
   Json::Value json_data;
   if (!json_reader.parse(buf, json_data)) {
-    errmsg = nacl::string("JSON parse error: ") +
+    std::string errmsg = nacl::string("JSON parse error: ") +
         json_reader.getFormatedErrorMessages();
+    coordinator_->ReportNonPpapiError(PP_NACL_ERROR_PNACL_RESOURCE_FETCH,
+        nacl::string("Parsing resource info failed: ") + errmsg + "\n");
     return false;
   }
 
   if (!json_data.isObject()) {
-    errmsg = nacl::string("Malformed JSON dictionary");
+    coordinator_->ReportNonPpapiError(PP_NACL_ERROR_PNACL_RESOURCE_FETCH,
+        nacl::string("Parsing resource info failed: "
+                     "Malformed JSON dictionary\n"));
     return false;
   }
 
   if (json_data.isMember("pnacl-llc-name")) {
     Json::Value json_name = json_data["pnacl-llc-name"];
     if (json_name.isString()) {
-      llc_tool_name = json_name.asString();
-      PLUGIN_PRINTF(("Set llc_tool_name=%s\n", llc_tool_name.c_str()));
+      llc_tool_name_ = json_name.asString();
+      PLUGIN_PRINTF(("Set llc_tool_name=%s\n", llc_tool_name_.c_str()));
     }
   }
 
   if (json_data.isMember("pnacl-ld-name")) {
     Json::Value json_name = json_data["pnacl-ld-name"];
     if (json_name.isString()) {
-      ld_tool_name = json_name.asString();
-      PLUGIN_PRINTF(("Set ld_tool_name=%s\n", ld_tool_name.c_str()));
+      ld_tool_name_ = json_name.asString();
+      PLUGIN_PRINTF(("Set ld_tool_name=%s\n", ld_tool_name_.c_str()));
     }
   }
-
   return true;
 }
 
 nacl::string PnaclResources::GetFullUrl(
     const nacl::string& partial_url, const nacl::string& sandbox_arch) const {
-  const nacl::string& url_with_platform_prefix =
-      sandbox_arch + "/" + partial_url;
-  nacl::string full_url = PnaclUrls::GetBaseUrl() + url_with_platform_prefix;
-  return full_url;
+  return PnaclUrls::GetBaseUrl() + sandbox_arch + "/" + partial_url;
 }
 
 void PnaclResources::StartLoad(
@@ -199,12 +193,6 @@ void PnaclResources::StartLoad(
   for (size_t i = 0; i < resource_urls.size(); ++i) {
     nacl::string full_url = GetFullUrl(
         resource_urls[i], plugin_->nacl_interface()->GetSandboxArch());
-    if (full_url == "") {
-      coordinator_->ReportNonPpapiError(
-          PP_NACL_ERROR_PNACL_RESOURCE_FETCH,
-          nacl::string("failed to resolve ") + resource_urls[i] + ".");
-      break;
-    }
     nacl::string filename = PnaclUrls::PnaclComponentURLToFilename(full_url);
 
     int32_t fd = PnaclResources::GetPnaclFD(plugin_, filename.c_str());

ppapi/native_client/src/trusted/plugin/pnacl_resources.h

@@ -35,11 +35,7 @@ class PnaclUrls {
 
   // Get the URL for the resource info JSON file that contains information
   // about loadable resources.
-  static const nacl::string GetResourceInfoUrl() {
-    return nacl::string(kResourceInfoUrl);
-  }
- private:
-  static const char kResourceInfoUrl[];
+  static nacl::string GetResourceInfoUrl();
 };
 
 // Loads a list of resources, providing a way to get file descriptors for
@@ -64,13 +60,8 @@ class PnaclResources {
   virtual void StartLoad(
       const pp::CompletionCallback& all_loaded_callback);
 
-  const nacl::string& GetLlcUrl() {
-    return llc_tool_name;
-  }
-
-  const nacl::string& GetLdUrl() {
-    return ld_tool_name;
-  }
+  const nacl::string& GetLlcUrl() { return llc_tool_name_; }
+  const nacl::string& GetLdUrl() { return ld_tool_name_; }
 
   nacl::string GetFullUrl(const nacl::string& partial_url,
                           const nacl::string& sandbox_arch) const;
@@ -93,16 +84,12 @@ class PnaclResources {
   std::map<nacl::string, nacl::DescWrapper*> resource_wrappers_;
 
   // Tool names for llc and ld; read from the resource info file.
-  nacl::string llc_tool_name;
-  nacl::string ld_tool_name;
+  nacl::string llc_tool_name_;
+  nacl::string ld_tool_name_;
 
   // Parses resource info json data in |buf|.  Returns true if successful.
-  // Otherwise returns false and places an error message in |errmsg|.
-  bool ParseResourceInfo(const nacl::string& buf, nacl::string& errmsg);
-
-  // Convenience function for reporting an error while reading the resource
-  // info file.
-  void ReadResourceInfoError(const nacl::string& msg);
+  // Otherwise returns false.
+  bool ParseResourceInfo(const nacl::string& buf);
 };
 
 }  // namespace plugin;