Pepper: Simplify error reporting in PnaclResources.

This change reduces the size of the public API for PnaclCoordinator and
simplifies error reporting inside PnaclResources, making it easier to follow.

BUG=239656
R=dmichael@chromium.org

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

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

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

@@ -377,7 +377,7 @@ void PnaclCoordinator::BitcodeStreamDidOpen(int32_t pp_error) {
   // The component updater's resource throttles + OnDemand update/install
   // should block the URL request until the compiler is present. Now we
   // can load the resources (e.g. llc and ld nexes).
-  resources_.reset(new PnaclResources(plugin_, this));
+  resources_.reset(new PnaclResources(plugin_));
   CHECK(resources_ != NULL);
 
   // The first step of loading resources: read the resource info file.
@@ -389,6 +389,10 @@ void PnaclCoordinator::BitcodeStreamDidOpen(int32_t pp_error) {
 void PnaclCoordinator::ResourceInfoWasRead(int32_t pp_error) {
   PLUGIN_PRINTF(("PluginCoordinator::ResourceInfoWasRead (pp_error=%"
                 NACL_PRId32 ")\n", pp_error));
+  if (pp_error != PP_OK) {
+    ExitWithError();
+    return;
+  }
   // Second step of loading resources: call StartLoad to load pnacl-llc
   // and pnacl-ld, based on the filenames found in the resource info file.
   pp::CompletionCallback resources_cb =
@@ -400,8 +404,11 @@ void PnaclCoordinator::ResourcesDidLoad(int32_t pp_error) {
   PLUGIN_PRINTF(("PnaclCoordinator::ResourcesDidLoad (pp_error=%"
                  NACL_PRId32 ")\n", pp_error));
   if (pp_error != PP_OK) {
-    // Finer-grained error code should have already been reported by
-    // the PnaclResources class.
+    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."));
     return;
   }
 

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

@@ -69,19 +69,6 @@ class PnaclCoordinator: public CallbackSource<FileStreamData> {
   // BitcodeToNative has completed (and the finish_callback called).
   PP_FileHandle TakeTranslatedFileHandle();
 
-  // Run |translate_notify_callback_| with an error condition that is not
-  // PPAPI specific.  Also set ErrorInfo report.
-  void ReportNonPpapiError(PP_NaClError err, const nacl::string& message);
-  // Run when faced with a PPAPI error condition. Bring control back to the
-  // plugin by invoking the |translate_notify_callback_|.
-  // Also set ErrorInfo report.
-  void ReportPpapiError(PP_NaClError err,
-                        int32_t pp_error, const nacl::string& message);
-  // Bring control back to the plugin by invoking the
-  // |translate_notify_callback_|.  This does not set the ErrorInfo report,
-  // it is assumed that it was already set.
-  void ExitWithError();
-
   // Implement FileDownloader's template of the CallbackSource interface.
   // This method returns a callback which will be called by the FileDownloader
   // to stream the bitcode data as it arrives. The callback
@@ -158,6 +145,20 @@ class PnaclCoordinator: public CallbackSource<FileStreamData> {
   void DoUMATimeMeasure(
       int32_t pp_error, const nacl::string& event_name, int64_t microsecs);
 
+  // Bring control back to the plugin by invoking the
+  // |translate_notify_callback_|.  This does not set the ErrorInfo report,
+  // it is assumed that it was already set.
+  void ExitWithError();
+  // Run |translate_notify_callback_| with an error condition that is not
+  // PPAPI specific.  Also set ErrorInfo report.
+  void ReportNonPpapiError(PP_NaClError err, const nacl::string& message);
+  // Run when faced with a PPAPI error condition. Bring control back to the
+  // plugin by invoking the |translate_notify_callback_|.
+  // Also set ErrorInfo report.
+  void ReportPpapiError(PP_NaClError err,
+                        int32_t pp_error, const nacl::string& message);
+
+
   // Keeps track of the pp_error upon entry to TranslateFinished,
   // for inspection after cleanup.
   int32_t translate_finish_error_;

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

@@ -9,7 +9,6 @@
 #include "native_client/src/trusted/desc/nacl_desc_wrapper.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/native_client/src/trusted/plugin/plugin.h"
-#include "ppapi/native_client/src/trusted/plugin/pnacl_coordinator.h"
 #include "ppapi/native_client/src/trusted/plugin/utility.h"
 
 namespace plugin {
@@ -41,9 +40,11 @@ void PnaclResources::ReadResourceInfo(
           "chrome://pnacl-translator/pnacl.json",
           &pp_llc_tool_name_var,
           &pp_ld_tool_name_var)) {
-    coordinator_->ExitWithError();
+    pp::Module::Get()->core()->CallOnMainThread(0,
+                                                resource_info_read_cb,
+                                                PP_ERROR_FAILED);
+    return;
   }
-
   pp::Var llc_tool_name(pp::PASS_REF, pp_llc_tool_name_var);
   pp::Var ld_tool_name(pp::PASS_REF, pp_ld_tool_name_var);
   llc_tool_name_ = GetFullUrl(llc_tool_name.AsString());
@@ -76,14 +77,6 @@ void PnaclResources::StartLoad(
   int32_t result = PP_OK;
   if (llc_file_handle_ == PP_kInvalidFileHandle ||
       ld_file_handle_ == PP_kInvalidFileHandle) {
-    // File-open failed. Assume this means that the file is
-    // not actually installed. This shouldn't actually occur since
-    // ReadResourceInfo() fail first.
-    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."));
     result = PP_ERROR_FILENOTFOUND;
   }
   pp::Module::Get()->core()->CallOnMainThread(0, all_loaded_callback, result);

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

@@ -20,17 +20,14 @@
 namespace plugin {
 
 class Plugin;
-class PnaclCoordinator;
 
 // Loads a list of resources, providing a way to get file descriptors for
 // these resources.  URLs for resources are resolved by the manifest
 // and point to pnacl component filesystem resources.
 class PnaclResources {
  public:
-  PnaclResources(Plugin* plugin,
-                 PnaclCoordinator* coordinator)
+  explicit PnaclResources(Plugin* plugin)
       : plugin_(plugin),
-        coordinator_(coordinator),
         llc_file_handle_(PP_kInvalidFileHandle),
         ld_file_handle_(PP_kInvalidFileHandle) {
   }
@@ -56,8 +53,6 @@ class PnaclResources {
 
   // The plugin requesting the resource loading.
   Plugin* plugin_;
-  // The coordinator responsible for reporting errors, etc.
-  PnaclCoordinator* coordinator_;
 
   // Tool names for llc and ld; read from the resource info file.
   nacl::string llc_tool_name_;