Pepper: Simplify OpenResource() for Non-SFI.

ppapi/native_client/src/trusted/plugin/service_runtime.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.
  */
 
 #define NACL_LOG_MODULE_NAME "Plugin_ServiceRuntime"
 
 #include "ppapi/native_client/src/trusted/plugin/service_runtime.h"
 
@@ skipping 33 matching lines @@
 #include "ppapi/native_client/src/trusted/plugin/plugin.h"
 #include "ppapi/native_client/src/trusted/plugin/plugin_error.h"
 #include "ppapi/native_client/src/trusted/plugin/pnacl_resources.h"
 #include "ppapi/native_client/src/trusted/plugin/sel_ldr_launcher_chrome.h"
 #include "ppapi/native_client/src/trusted/plugin/srpc_client.h"
 #include "ppapi/native_client/src/trusted/plugin/utility.h"
 #include "ppapi/native_client/src/trusted/weak_ref/call_on_main_thread.h"
 
 namespace plugin {
 
-class OpenManifestEntryAsyncCallback {
- public:
-  OpenManifestEntryAsyncCallback(PP_OpenResourceCompletionCallback callback,
-                                 void* callback_user_data)
-      : callback_(callback), callback_user_data_(callback_user_data) {
-  }
-
-  ~OpenManifestEntryAsyncCallback() {
-    if (callback_)
-      callback_(callback_user_data_, PP_kInvalidFileHandle);
-  }
-
-  void Run(int32_t pp_error) {
-#if defined(OS_WIN)
-    // Currently, this is used only for non-SFI mode, and now the mode is not
-    // supported on windows.
-    // TODO(hidehiko): Support it on Windows when we switch to use
-    // ManifestService also in SFI-mode.
-    NACL_NOTREACHED();
-#elif defined(OS_POSIX)
-    // On posix, PlatformFile is the file descriptor.
-    callback_(callback_user_data_, (pp_error == PP_OK) ? info_.desc : -1);
-    callback_ = NULL;
-#endif
-  }
-
-  NaClFileInfo* mutable_info() { return &info_; }
-
- private:
-  NaClFileInfo info_;
-  PP_OpenResourceCompletionCallback callback_;
-  void* callback_user_data_;
-  DISALLOW_COPY_AND_ASSIGN(OpenManifestEntryAsyncCallback);
-};
-
 namespace {
 
 class ManifestService {
  public:
   ManifestService(nacl::WeakRefAnchor* anchor,
                   PluginReverseInterface* plugin_reverse)
       : anchor_(anchor),
         plugin_reverse_(plugin_reverse) {
   }
 
@@ skipping 10 matching lines @@
     // Release this instance if the ServiceRuntime is already destructed.
     if (anchor_->is_abandoned()) {
       delete this;
       return false;
     }
 
     plugin_reverse_->StartupInitializationComplete();
     return true;
   }
 
-  bool OpenResource(const char* entry_key,
-                    PP_OpenResourceCompletionCallback callback,
-                    void* callback_user_data) {
-    // Release this instance if the ServiceRuntime is already destructed.
-    if (anchor_->is_abandoned()) {
-      callback(callback_user_data, PP_kInvalidFileHandle);
-      delete this;
-      return false;
-    }
-
-    OpenManifestEntryAsyncCallback* open_manifest_callback =
-        new OpenManifestEntryAsyncCallback(callback, callback_user_data);
-    plugin_reverse_->OpenManifestEntryAsync(
-        entry_key,
-        open_manifest_callback->mutable_info(),
-        open_manifest_callback);
-    return true;
-  }
-
   static PP_Bool QuitTrampoline(void* user_data) {
     return PP_FromBool(static_cast<ManifestService*>(user_data)->Quit());
   }
 
   static PP_Bool StartupInitializationCompleteTrampoline(void* user_data) {
     return PP_FromBool(static_cast<ManifestService*>(user_data)->
                        StartupInitializationComplete());
   }
 
-  static PP_Bool OpenResourceTrampoline(
-      void* user_data,
-      const char* entry_key,
-      PP_OpenResourceCompletionCallback callback,
-      void* callback_user_data) {
-    return PP_FromBool(static_cast<ManifestService*>(user_data)->OpenResource(
-        entry_key, callback, callback_user_data));
-  }
-
  private:
   // Weak reference to check if plugin_reverse is legally accessible or not.
   nacl::WeakRefAnchor* anchor_;
   PluginReverseInterface* plugin_reverse_;
 
   DISALLOW_COPY_AND_ASSIGN(ManifestService);
 };
 
 // Vtable to pass functions to LaunchSelLdr.
 const PPP_ManifestService kManifestServiceVTable = {
   &ManifestService::QuitTrampoline,
   &ManifestService::StartupInitializationCompleteTrampoline,
-  &ManifestService::OpenResourceTrampoline,
+  NULL

[dmichael (off chromium), 2014/06/17 21:52:28]

Can we remove that from PPP_ManifestService in this CL?

[teravest, 2014/06/18 20:13:44]

Done.

 };
 
 }  // namespace
 
 OpenManifestEntryResource::~OpenManifestEntryResource() {
-  MaybeRunCallback(PP_ERROR_ABORTED);
-}
-
-void OpenManifestEntryResource::MaybeRunCallback(int32_t pp_error) {
-  if (!callback)
-    return;
-
-  callback->Run(pp_error);
-  delete callback;
-  callback = NULL;
 }
 
 PluginReverseInterface::PluginReverseInterface(
     nacl::WeakRefAnchor* anchor,
     Plugin* plugin,
     ServiceRuntime* service_runtime,
     pp::CompletionCallback init_done_cb,
     pp::CompletionCallback crash_cb)
       : anchor_(anchor),
         plugin_(plugin),
@@ skipping 42 matching lines @@
 // and invoke StreamAsFile with a completion callback that invokes
 // GetPOSIXFileDesc.
 bool PluginReverseInterface::OpenManifestEntry(nacl::string url_key,
                                                struct NaClFileInfo* info) {
   bool op_complete = false;  // NB: mu_ and cv_ also controls access to this!
   // The to_open object is owned by the weak ref callback. Because this function
   // waits for the callback to finish, the to_open object will be deallocated on
   // the main thread before this function can return. The pointers it contains
   // to stack variables will not leak.
   OpenManifestEntryResource* to_open =
-      new OpenManifestEntryResource(url_key, info, &op_complete, NULL);
+      new OpenManifestEntryResource(url_key, info, &op_complete);
   CHECK(to_open != NULL);
   NaClLog(4, "PluginReverseInterface::OpenManifestEntry: %s\n",
           url_key.c_str());
   // This assumes we are not on the main thread.  If false, we deadlock.
   plugin::WeakRefCallOnMainThread(
       anchor_,
       0,
       this,
       &plugin::PluginReverseInterface::OpenManifestEntry_MainThreadContinuation,
       to_open);
@@ skipping 29 matching lines @@
   if (info->desc == -1) {
     // TODO(bsy,ncbray): what else should we do with the error?  This
     // is a runtime error that may simply be a programming error in
     // the untrusted code, or it may be something else wrong w/ the
     // manifest.
     NaClLog(4, "OpenManifestEntry: failed for key %s", url_key.c_str());
   }
   return true;
 }
 
-void PluginReverseInterface::OpenManifestEntryAsync(
-    const nacl::string& entry_key,
-    struct NaClFileInfo* info,
-    OpenManifestEntryAsyncCallback* callback) {
-  bool op_complete = false;
-  OpenManifestEntryResource to_open(
-      entry_key, info, &op_complete, callback);
-  OpenManifestEntry_MainThreadContinuation(&to_open, PP_OK);
-}
-
 // Transfer point from OpenManifestEntry() which runs on the main thread
 // (Some PPAPI actions -- like StreamAsFile -- can only run on the main thread).
 // OpenManifestEntry() is waiting on a condvar for this continuation to
 // complete.  We Broadcast and awaken OpenManifestEntry() whenever we are done
 // either here, or in a later MainThreadContinuation step, if there are
 // multiple steps.
 void PluginReverseInterface::OpenManifestEntry_MainThreadContinuation(
     OpenManifestEntryResource* p,
     int32_t err) {
   UNREFERENCED_PARAMETER(err);
@@ skipping 11 matching lines @@
           &pnacl_options)) {
     NaClLog(4, "OpenManifestEntry_MainThreadContinuation: ResolveKey failed\n");
     // Failed, and error_info has the details on what happened.  Wake
     // up requesting thread -- we are done.
     {
       nacl::MutexLocker take(&mu_);
       *p->op_complete_ptr = true;  // done...
       p->file_info->desc = -1;  // but failed.
       NaClXCondVarBroadcast(&cv_);
     }
-    p->MaybeRunCallback(PP_OK);
     return;
   }
   nacl::string mapped_url = pp::Var(pp_mapped_url).AsString();
   NaClLog(4,
           "OpenManifestEntry_MainThreadContinuation: "
           "ResolveKey: %s -> %s (pnacl_translate(%d))\n",
           p->url.c_str(), mapped_url.c_str(), pnacl_options.translate);
 
   if (pnacl_options.translate) {
     // Requires PNaCl translation, but that's not supported.
     NaClLog(4,
             "OpenManifestEntry_MainThreadContinuation: "
             "Requires PNaCl translation -- not supported\n");
     {
       nacl::MutexLocker take(&mu_);
       *p->op_complete_ptr = true;  // done...
       p->file_info->desc = -1;  // but failed.
       NaClXCondVarBroadcast(&cv_);
     }
-    p->MaybeRunCallback(PP_OK);
     return;
   }
 
   // Because p is owned by the callback of this invocation, so it is necessary
   // to create another instance.
   OpenManifestEntryResource* open_cont = new OpenManifestEntryResource(*p);
   open_cont->url = mapped_url;
-  // Callback is now delegated from p to open_cont. So, here we manually clear
-  // complete callback.
-  p->callback = NULL;
 
   pp::CompletionCallback stream_cc = WeakRefNewCallback(
       anchor_,
       this,
       &PluginReverseInterface::StreamAsFile_MainThreadContinuation,
       open_cont);
 
   GetNaClInterface()->DownloadFile(plugin_->pp_instance(),
                                    mapped_url.c_str(),
                                    &open_cont->pp_file_info,
@@ skipping 18 matching lines @@
               p->file_info->desc);
     } else {
       NaClLog(
           4,
           "StreamAsFile_MainThreadContinuation: !PP_OK, setting desc -1\n");
       p->file_info->desc = -1;
     }
     *p->op_complete_ptr = true;
     NaClXCondVarBroadcast(&cv_);
   }
-  p->MaybeRunCallback(PP_OK);
 }
 
 bool PluginReverseInterface::CloseManifestEntry(int32_t desc) {
   // We don't take any action on a call to CloseManifestEntry today, so always
   // return success.
   return true;
 }
 
 void PluginReverseInterface::ReportCrash() {
   NaClLog(4, "PluginReverseInterface::ReportCrash\n");
@@ skipping 442 matching lines @@
 
 nacl::string ServiceRuntime::GetCrashLogOutput() {
   if (NULL != subprocess_.get()) {
     return subprocess_->GetCrashLogOutput();
   } else {
     return std::string();
   }
 }
 
 }  // namespace plugin