Trusted plugin: Miscellaneous cleanup.

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

 // Copyright (c) 2012 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.
 
 #ifdef _MSC_VER
 // Do not warn about use of std::copy with raw pointers.
 #pragma warning(disable : 4996)
 #endif
 
 #include "ppapi/native_client/src/trusted/plugin/plugin.h"
@@ skipping 336 matching lines @@
     return NULL;
   }
 
   PLUGIN_PRINTF(("Plugin::LoadHelperNaClModule (%s, %s)\n",
                  helper_url.c_str(),
                  nacl_subprocess.get()->detailed_description().c_str()));
 
   return nacl_subprocess.release();
 }
 
-Plugin* Plugin::New(PP_Instance pp_instance) {
-  PLUGIN_PRINTF(("Plugin::New (pp_instance=%" NACL_PRId32 ")\n", pp_instance));
-  Plugin* plugin = new Plugin(pp_instance);
-  PLUGIN_PRINTF(("Plugin::New (plugin=%p)\n", static_cast<void*>(plugin)));
-  return plugin;
-}
-
 // All failures of this function will show up as "Missing Plugin-in", so
 // there is no need to log to JS console that there was an initialization
 // failure. Note that module loading functions will log their own errors.
 bool Plugin::Init(uint32_t argc, const char* argn[], const char* argv[]) {
   PLUGIN_PRINTF(("Plugin::Init (argc=%" NACL_PRIu32 ")\n", argc));
   nacl_interface_->InitializePlugin(pp_instance(), argc, argn, argv);
-  url_util_ = pp::URLUtil_Dev::Get();
-  if (url_util_ == NULL)
-    return false;
-
   wrapper_factory_ = new nacl::DescWrapperFactory();
   pp::Var manifest_url(pp::PASS_REF, nacl_interface_->GetManifestURLArgument(
       pp_instance()));
   if (manifest_url.is_string() && !manifest_url.AsString().empty())
     RequestNaClManifest(manifest_url.AsString());
   return true;
 }
 
 Plugin::Plugin(PP_Instance pp_instance)
     : pp::Instance(pp_instance),
@@ skipping 263 matching lines @@
     ReportLoadError(error_info);
     return;
   }
 
   ProcessNaClManifest(json_buffer);
 }
 
 void Plugin::ProcessNaClManifest(const nacl::string& manifest_json) {
   HistogramSizeKB("NaCl.Perf.Size.Manifest",
                   static_cast<int32_t>(manifest_json.length() / 1024));
-  ErrorInfo error_info;
-  if (!SetManifestObject(manifest_json, &error_info)) {
-    ReportLoadError(error_info);
+  if (!SetManifestObject(manifest_json))
     return;
-  }
 
   nacl::string program_url;
   PP_PNaClOptions pnacl_options = {PP_FALSE, PP_FALSE, 2};
   bool uses_nonsfi_mode;
+  ErrorInfo error_info;
   if (manifest_->GetProgramURL(
           &program_url, &pnacl_options, &uses_nonsfi_mode, &error_info)) {
     // TODO(teravest): Make ProcessNaClManifest take responsibility for more of
     // this function.
     nacl_interface_->ProcessNaClManifest(pp_instance(), program_url.c_str());
     uses_nonsfi_mode_ = uses_nonsfi_mode;
     if (pnacl_options.translate) {
       pp::CompletionCallback translate_callback =
           callback_factory_.NewCallback(&Plugin::BitcodeDidTranslate);
       // Will always call the callback on success or failure.
@@ skipping 59 matching lines @@
     // Will always call the callback on success or failure.
     CHECK(nexe_downloader_.Open(nmf_resolved_url.AsString(),
                                 DOWNLOAD_TO_FILE,
                                 open_callback,
                                 false,
                                 NULL));
   }
 }
 
 
-bool Plugin::SetManifestObject(const nacl::string& manifest_json,
-                               ErrorInfo* error_info) {
+bool Plugin::SetManifestObject(const nacl::string& manifest_json) {
   PLUGIN_PRINTF(("Plugin::SetManifestObject(): manifest_json='%s'.\n",
        manifest_json.c_str()));
-  if (error_info == NULL)
-    return false;
+  ErrorInfo error_info;
+
   // Determine whether lookups should use portable (i.e., pnacl versions)
   // rather than platform-specific files.
   bool is_pnacl = nacl_interface_->IsPNaCl(pp_instance());
   bool nonsfi_mode_enabled =
       PP_ToBool(nacl_interface_->IsNonSFIModeEnabled());
   pp::Var manifest_base_url =
       pp::Var(pp::PASS_REF, nacl_interface_->GetManifestBaseURL(pp_instance()));
   std::string manifest_base_url_str = manifest_base_url.AsString();
   bool pnacl_debug = GetNaClInterface()->NaClDebugEnabledForURL(
       manifest_base_url_str.c_str());
   const char* sandbox_isa = nacl_interface_->GetSandboxArch();
   nacl::scoped_ptr<JsonManifest> json_manifest(
-      new JsonManifest(url_util_,
+      new JsonManifest(pp::URLUtil_Dev::Get(),
                        manifest_base_url_str,
                        (is_pnacl ? kPortableArch : sandbox_isa),
                        nonsfi_mode_enabled,
                        pnacl_debug));
-  if (!json_manifest->Init(manifest_json, error_info)) {
+  if (!json_manifest->Init(manifest_json, &error_info)) {
+    ReportLoadError(error_info);
     return false;
   }
   manifest_.reset(json_manifest.release());
   return true;
 }
 
 void Plugin::UrlDidOpenForStreamAsFile(
     int32_t pp_error,
     FileDownloader* url_downloader,
     pp::CompletionCallback callback) {
   PLUGIN_PRINTF(("Plugin::UrlDidOpen (pp_error=%" NACL_PRId32
                  ", url_downloader=%p)\n", pp_error,
                  static_cast<void*>(url_downloader)));
   url_downloaders_.erase(url_downloader);
   nacl::scoped_ptr<FileDownloader> scoped_url_downloader(url_downloader);
   NaClFileInfo tmp_info(scoped_url_downloader->GetFileInfo());
   NaClFileInfoAutoCloser *info = new NaClFileInfoAutoCloser(&tmp_info);
 
   if (pp_error != PP_OK) {
     callback.Run(pp_error);
     delete info;
   } else if (info->get_desc() > NACL_NO_FILE_DESC) {
     std::map<nacl::string, NaClFileInfoAutoCloser*>::iterator it =
         url_file_info_map_.find(url_downloader->url());
-    if (it != url_file_info_map_.end()) {
+    if (it != url_file_info_map_.end())
       delete it->second;
-    }
     url_file_info_map_[url_downloader->url()] = info;
     callback.Run(PP_OK);
   } else {
     callback.Run(PP_ERROR_FAILED);
     delete info;
   }
 }
 
 struct NaClFileInfo Plugin::GetFileInfo(const nacl::string& url) {
   struct NaClFileInfo info;
@@ skipping 161 matching lines @@
   // We shouldn't hit this if the file URL is in an installed app.
   if (file_handle == PP_kInvalidFileHandle)
     return false;
 
   // FileDownloader takes ownership of the file handle.
   downloader->OpenFast(url, file_handle, file_token_lo, file_token_hi);
   return true;
 }
 
 bool Plugin::DocumentCanRequest(const std::string& url) {
-  CHECK(url_util_ != NULL);
-  return url_util_->DocumentCanRequest(this, pp::Var(url));
+  CHECK(pp::URLUtil_Dev::Get() != NULL);
+  return pp::URLUtil_Dev::Get()->DocumentCanRequest(this, pp::Var(url));

[dmichael (off chromium), 2014/04/28 23:01:44]

Hopefully this doesn't matter, but *if* this ever gets invoked on a background
thread, it's more likely to fail now. Though it was probably never thread-safe.

Might be worth adding a check that you're on the main thread.

 }
 
 void Plugin::set_exit_status(int exit_status) {
   pp::Core* core = pp::Module::Get()->core();
   if (core->IsMainThread()) {
     SetExitStatusOnMainThread(PP_OK, exit_status);
   } else {
     pp::CompletionCallback callback =
         callback_factory_.NewCallback(&Plugin::SetExitStatusOnMainThread,
                                       exit_status);
     core->CallOnMainThread(0, callback, 0);
   }
 }
 
 void Plugin::SetExitStatusOnMainThread(int32_t pp_error,
                                        int exit_status) {
   DCHECK(pp::Module::Get()->core()->IsMainThread());
   DCHECK(nacl_interface_);
   nacl_interface_->SetExitStatus(pp_instance(), exit_status);
 }
 
 
 }  // namespace plugin