Pepper: Delete FileDownloader in trusted plugin.

ppapi/native_client/src/trusted/plugin/pnacl_coordinator.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.
 
 #include "ppapi/native_client/src/trusted/plugin/pnacl_coordinator.h"
 
 #include <utility>
 #include <vector>
 
 #include "native_client/src/include/checked_cast.h"
@@ skipping 45 matching lines @@
   uma.HistogramEnumeration("NaCl.Perf.PNaClCache.IsHit",
                            hit, 2);
 }
 
 nacl::string GetArchitectureAttributes(Plugin* plugin) {
   pp::Var attrs_var(pp::PASS_REF,
                     plugin->nacl_interface()->GetCpuFeatureAttrs());
   return attrs_var.AsString();
 }
 
+void DidCacheHit(void* user_data, PP_FileHandle nexe_file_handle) {
+  PnaclCoordinator* coordinator = static_cast<PnaclCoordinator*>(user_data);
+  coordinator->BitcodeStreamCacheHit(nexe_file_handle);
+}
+
+void DidCacheMiss(void* user_data, int64_t expected_pexe_size) {
+  PnaclCoordinator* coordinator = static_cast<PnaclCoordinator*>(user_data);
+  coordinator->BitcodeStreamCacheMiss(expected_pexe_size);
+  // Do some fixup for temp_nexe_file_ here.

[dmichael (off chromium), 2014/07/17 16:13:21]

I don't understand this comment; was there something else you wanted to do here
before landing?

[teravest, 2014/07/21 18:33:21]

Removed.

+}
+
+void DidStreamData(void* user_data, const void* stream_data, int32_t length) {
+  PnaclCoordinator* coordinator = static_cast<PnaclCoordinator*>(user_data);
+  coordinator->BitcodeStreamGotData(stream_data, length);
+}
+
+void DidFinishStream(void* user_data, int32_t pp_error) {
+  PnaclCoordinator* coordinator = static_cast<PnaclCoordinator*>(user_data);
+  coordinator->BitcodeStreamDidFinish(pp_error);
+  if (pp_error != PP_OK) {
+

[dmichael (off chromium), 2014/07/17 16:13:21]

Is this obsolete? You're presumably handling this in the PnaclCoordinator
function...

[teravest, 2014/07/21 18:33:21]

Yes, removed.

+  }
+}
+
+PPP_PexeStreamHandler kPexeStreamHandler = {
+  &DidCacheHit,
+  &DidCacheMiss,
+  &DidStreamData,
+  &DidFinishStream
+};
+
 }  // namespace
 
-// Out-of-line destructor to keep it from getting put in every .o where
-// callback_source.h is included
-template<>
-CallbackSource<FileStreamData>::~CallbackSource() {}
-
 PnaclCoordinator* PnaclCoordinator::BitcodeToNative(
     Plugin* plugin,
     const nacl::string& pexe_url,
     const PP_PNaClOptions& pnacl_options,
     const pp::CompletionCallback& translate_notify_callback) {
   PLUGIN_PRINTF(("PnaclCoordinator::BitcodeToNative (plugin=%p, pexe=%s)\n",
                  static_cast<void*>(plugin), pexe_url.c_str()));
   PnaclCoordinator* coordinator =
       new PnaclCoordinator(plugin, pexe_url,
                            pnacl_options,
@@ skipping 17 matching lines @@
     const PP_PNaClOptions& pnacl_options,
     const pp::CompletionCallback& translate_notify_callback)
   : translate_finish_error_(PP_OK),
     plugin_(plugin),
     translate_notify_callback_(translate_notify_callback),
     translation_finished_reported_(false),
     pexe_url_(pexe_url),
     pnacl_options_(pnacl_options),
     architecture_attributes_(GetArchitectureAttributes(plugin)),
     split_module_count_(1),
-    is_cache_hit_(PP_FALSE),
     error_already_reported_(false),
     pexe_size_(0),
     pexe_bytes_compiled_(0),
     expected_pexe_size_(-1) {
   PLUGIN_PRINTF(("PnaclCoordinator::PnaclCoordinator (this=%p, plugin=%p)\n",
                  static_cast<void*>(this), static_cast<void*>(plugin)));
   callback_factory_.Initialize(this);
 }
 
 PnaclCoordinator::~PnaclCoordinator() {
@@ skipping 137 matching lines @@
     ReportPpapiError(PP_NACL_ERROR_PNACL_CACHE_FETCH_OTHER,
                      pp_error,
                      "Failed to open translated nexe.");
     return;
   }
 
   translate_notify_callback_.Run(pp_error);
 }
 
 void PnaclCoordinator::OpenBitcodeStream() {
-  // Now open the pexe stream.
-  streaming_downloader_.reset(new FileDownloader(plugin_));
-  // Mark the request as requesting a PNaCl bitcode file,
-  // so that component updater can detect this user action.
-  streaming_downloader_->set_request_headers(
-      "Accept: application/x-pnacl, */*");
-
-  // Even though we haven't started downloading, create the translation
-  // thread object immediately. This ensures that any pieces of the file
-  // that get downloaded before the compilation thread is accepting
-  // SRPCs won't get dropped.
-  translate_thread_.reset(new PnaclTranslateThread());
-  if (translate_thread_ == NULL) {
-    ReportNonPpapiError(
-        PP_NACL_ERROR_PNACL_THREAD_CREATE,
-        "PnaclCoordinator: could not allocate translation thread.");
-    return;
-  }
-
-  pp::CompletionCallback cb =
-      callback_factory_.NewCallback(&PnaclCoordinator::BitcodeStreamDidOpen);
-  if (!streaming_downloader_->OpenStream(pexe_url_, cb, this)) {
-    ReportNonPpapiError(
-        PP_NACL_ERROR_PNACL_PEXE_FETCH_OTHER,
-        nacl::string("PnaclCoordinator: failed to open stream ") + pexe_url_);
-    return;
-  }
-}
-
-void PnaclCoordinator::BitcodeStreamDidOpen(int32_t pp_error) {
-  if (pp_error != PP_OK) {
-    BitcodeStreamDidFinish(pp_error);
-    // We have not spun up the translation process yet, so we need to call
-    // TranslateFinished here.
-    TranslateFinished(pp_error);
-    return;
-  }
-
   // 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_));
   CHECK(resources_ != NULL);

[dmichael (off chromium), 2014/07/17 16:13:21]

I'm a little confused by this. Are you sure your change works in the case of a
Chrome that hasn't received the PNaCl llc/ld stuff via component updater yet? I
don't *think* we can ignore that case yet, and I'm not sure how fetching that
stuff is initiated.

[teravest, 2014/07/21 18:33:21]

What are you confused about here? This shouldn't change the behavior for the
availability of llc and ld; the callback that invoked BitcodeStreamDidOpen()
previously was based on opening the pexe stream.

[dmichael (off chromium), 2014/07/21 19:49:22]

Sorry, I was worried that somehow the PnaclTranslateThread creation was
responsible for the fetching of llc and ld, and you moved that later. I assumed
(without looking) that this new PnaclResources would fail if llc and ld didn't
exist or something. It looks like it's fine and llc/ld are actually fetched in
resource_->StartLoad, so I was worried for nothing.

Nevermind.

 
   // The first step of loading resources: read the resource info file.
   if (!resources_->ReadResourceInfo()) {
     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.
   if (!resources_->StartLoad()) {
     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;
   }
 
-  // Okay, now that we've started the HTTP request for the pexe
-  // and we've ensured that the PNaCl compiler + metadata is installed,
-  // get the cache key from the response headers and from the
-  // compiler's version metadata.
-  nacl::string headers = streaming_downloader_->GetResponseHeaders();
+  // Even though we haven't started downloading, create the translation
+  // thread object immediately. This ensures that any pieces of the file
+  // that get downloaded before the compilation thread is accepting
+  // SRPCs won't get dropped.
+  translate_thread_.reset(new PnaclTranslateThread());
+  if (translate_thread_ == NULL) {
+    ReportNonPpapiError(
+        PP_NACL_ERROR_PNACL_THREAD_CREATE,
+        "PnaclCoordinator: could not allocate translation thread.");
+    return;
+  }
 
-  temp_nexe_file_.reset(new TempFile(plugin_));
-  pp::CompletionCallback cb =
-      callback_factory_.NewCallback(&PnaclCoordinator::NexeFdDidOpen);
-  int32_t nexe_fd_err =
-      plugin_->nacl_interface()->GetNexeFd(
-          plugin_->pp_instance(),
-          streaming_downloader_->full_url().c_str(),
-          // TODO(dschuff): Get this value from the pnacl json file after it
-          // rolls in from NaCl.
-          1,
-          pnacl_options_.opt_level,
-          headers.c_str(),
-          architecture_attributes_.c_str(), // Extra compile flags.
-          &is_cache_hit_,
-          temp_nexe_file_->internal_handle(),
-          cb.pp_completion_callback());
-  if (nexe_fd_err < PP_OK_COMPLETIONPENDING) {
-    ReportPpapiError(PP_NACL_ERROR_PNACL_CREATE_TEMP, nexe_fd_err,
-                     nacl::string("Call to GetNexeFd failed"));
+  GetNaClInterface()->StreamPexe(plugin_->pp_instance(),
+                                 pexe_url_.c_str(),
+                                 pnacl_options_.opt_level,
+                                 &kPexeStreamHandler,
+                                 this);
+}
+
+void PnaclCoordinator::BitcodeStreamCacheHit(PP_FileHandle handle) {
+  HistogramEnumerateTranslationCache(plugin_->uma_interface(), true);
+  if (handle == PP_kInvalidFileHandle) {
+    ReportNonPpapiError(
+        PP_NACL_ERROR_PNACL_CREATE_TEMP,
+        nacl::string(
+            "PnaclCoordinator: Got bad temp file handle from GetNexeFd"));
+    BitcodeStreamDidFinish(PP_ERROR_FAILED);
+    return;
   }
+  *temp_nexe_file_->internal_handle() = handle;

[dmichael (off chromium), 2014/07/17 16:13:21]

nit: This looks a bit weird to me; I think I'd prefer a public member or a
setter. (Ignore if you want since it's existing code... that will go away)

+  // Open it for reading as the cached nexe file.
+  NexeReadDidOpen(temp_nexe_file_->Open(false));
+}
+
+void PnaclCoordinator::BitcodeStreamCacheMiss(int64_t expected_pexe_size) {
+  HistogramEnumerateTranslationCache(plugin_->uma_interface(), false);
+  expected_pexe_size_ = expected_pexe_size;
+  NexeFdDidOpen(PP_OK);
+}
+
+void PnaclCoordinator::BitcodeStreamGotData(const void* data, int32_t length) {
+  DCHECK(translate_thread_.get());
+
+  translate_thread_->PutBytes(data, length);
+  if (data && length > 0)
+    pexe_size_ += length;
 }
 
 void PnaclCoordinator::NexeFdDidOpen(int32_t pp_error) {
-  PLUGIN_PRINTF(("PnaclCoordinator::NexeFdDidOpen (pp_error=%"
-                 NACL_PRId32 ", hit=%d)\n", pp_error,
-                 is_cache_hit_ == PP_TRUE));
   if (pp_error < PP_OK) {
     ReportPpapiError(PP_NACL_ERROR_PNACL_CREATE_TEMP, pp_error,
                      nacl::string("GetNexeFd failed"));
     return;
   }
 
-  if (*temp_nexe_file_->internal_handle() == PP_kInvalidFileHandle) {
-    ReportNonPpapiError(
-        PP_NACL_ERROR_PNACL_CREATE_TEMP,
-        nacl::string(
-            "PnaclCoordinator: Got bad temp file handle from GetNexeFd"));
-    return;
+
+  // Open an object file first so the translator can start writing to it
+  // during streaming translation.
+  temp_nexe_file_.reset(new TempFile(plugin_));
+
+  for (int i = 0; i < split_module_count_; i++) {
+    nacl::scoped_ptr<TempFile> temp_file(new TempFile(plugin_));
+    int32_t pp_error = temp_file->Open(true);
+    if (pp_error != PP_OK) {
+      ReportPpapiError(PP_NACL_ERROR_PNACL_CREATE_TEMP,
+                       pp_error,
+                       "Failed to open scratch object file.");
+      return;
+    } else {
+      obj_files_.push_back(temp_file.release());
+    }
   }
-  HistogramEnumerateTranslationCache(plugin_->uma_interface(), is_cache_hit_);
+  invalid_desc_wrapper_.reset(plugin_->wrapper_factory()->MakeInvalid());
 
-  if (is_cache_hit_ == PP_TRUE) {
-    // Cache hit -- no need to stream the rest of the file.
-    streaming_downloader_.reset(NULL);
-    // Open it for reading as the cached nexe file.
-    NexeReadDidOpen(temp_nexe_file_->Open(false));
-  } else {
-    // Open an object file first so the translator can start writing to it
-    // during streaming translation.
-    for (int i = 0; i < split_module_count_; i++) {
-      nacl::scoped_ptr<TempFile> temp_file(new TempFile(plugin_));
-      int32_t pp_error = temp_file->Open(true);
-      if (pp_error != PP_OK) {
-        ReportPpapiError(PP_NACL_ERROR_PNACL_CREATE_TEMP,
-                         pp_error,
-                         "Failed to open scratch object file.");
-        return;
-      } else {
-        obj_files_.push_back(temp_file.release());
-      }
-    }
-    invalid_desc_wrapper_.reset(plugin_->wrapper_factory()->MakeInvalid());
-
-    // Meanwhile, a miss means we know we need to stream the bitcode, so stream
-    // the rest of it now. (Calling BeginStreaming means that the downloader
-    // will begin handing data to the coordinator, which is safe any time after
-    // the translate_thread_ object has been initialized).
-    pp::CompletionCallback finish_cb = callback_factory_.NewCallback(
-        &PnaclCoordinator::BitcodeStreamDidFinish);
-    streaming_downloader_->BeginStreaming(finish_cb);
-
-    // Open the nexe file for connecting ld and sel_ldr.
-    // Start translation when done with this last step of setup!
-    RunTranslate(temp_nexe_file_->Open(true));
-  }
+  // Open the nexe file for connecting ld and sel_ldr.
+  // Start translation when done with this last step of setup!
+  RunTranslate(temp_nexe_file_->Open(true));
 }
 
 void PnaclCoordinator::BitcodeStreamDidFinish(int32_t pp_error) {
   PLUGIN_PRINTF(("PnaclCoordinator::BitcodeStreamDidFinish (pp_error=%"
                  NACL_PRId32 ")\n", pp_error));
   if (pp_error != PP_OK) {
     // Defer reporting the error and cleanup until after the translation
     // thread returns, because it may be accessing the coordinator's
     // objects or writing to the files.
     translate_finish_error_ = pp_error;
     if (pp_error == PP_ERROR_ABORTED) {
       error_info_.SetReport(PP_NACL_ERROR_PNACL_PEXE_FETCH_ABORTED,
                             "PnaclCoordinator: pexe load failed (aborted).");
     }
     if (pp_error == PP_ERROR_NOACCESS) {
       error_info_.SetReport(PP_NACL_ERROR_PNACL_PEXE_FETCH_NOACCESS,
                             "PnaclCoordinator: pexe load failed (no access).");
     } else {
       nacl::stringstream ss;
       ss << "PnaclCoordinator: pexe load failed (pp_error=" << pp_error << ").";
       error_info_.SetReport(PP_NACL_ERROR_PNACL_PEXE_FETCH_OTHER, ss.str());
     }
-    translate_thread_->AbortSubprocesses();
+
+    if (translate_thread_->started())
+      translate_thread_->AbortSubprocesses();
+    else
+      TranslateFinished(pp_error);
   } else {
     // Compare download completion pct (100% now), to compile completion pct.
     HistogramRatio(plugin_->uma_interface(),
                    "NaCl.Perf.PNaClLoadTime.PctCompiledWhenFullyDownloaded",
                    pexe_bytes_compiled_, pexe_size_);
-  }
-}
-
-void PnaclCoordinator::BitcodeStreamGotData(int32_t pp_error,
-                                            FileStreamData data) {
-  PLUGIN_PRINTF(("PnaclCoordinator::BitcodeStreamGotData (pp_error=%"
-                 NACL_PRId32 ", data=%p)\n", pp_error, data ? &(*data)[0] : 0));
-  DCHECK(translate_thread_.get());
-
-  // When we have received data, pp_error is set to the number of bytes
-  // received.
-  if (pp_error > 0) {
-    CHECK(data);
-    translate_thread_->PutBytes(data, pp_error);
-    pexe_size_ += pp_error;
-  } else {
     translate_thread_->EndStream();
   }
 }
 
-StreamCallback PnaclCoordinator::GetCallback() {
-  return callback_factory_.NewCallbackWithOutput(
-      &PnaclCoordinator::BitcodeStreamGotData);
-}
-
 void PnaclCoordinator::BitcodeGotCompiled(int32_t pp_error,
                                           int64_t bytes_compiled) {
   DCHECK(pp_error == PP_OK);
   pexe_bytes_compiled_ += bytes_compiled;
   // If we don't know the expected total yet, ask.

[dmichael (off chromium), 2014/07/17 16:13:21]

obsolete comment?

-  if (expected_pexe_size_ == -1) {
-    int64_t amount_downloaded;  // dummy variable.
-    streaming_downloader_->GetDownloadProgress(&amount_downloaded,
-                                               &expected_pexe_size_);
-  }
+
   // Hold off reporting the last few bytes of progress, since we don't know
   // when they are actually completely compiled.  "bytes_compiled" only means
   // that bytes were sent to the compiler.
   if (expected_pexe_size_ != -1) {
     if (!ShouldDelayProgressEvent()) {
       GetNaClInterface()->DispatchEvent(plugin_->pp_instance(),
                                         PP_NACL_EVENT_PROGRESS,
                                         pexe_url_.c_str(),
                                         PP_TRUE,
                                         pexe_bytes_compiled_,
@@ skipping 36 matching lines @@
                                   invalid_desc_wrapper_.get(),
                                   &error_info_,
                                   resources_.get(),
                                   &pnacl_options_,
                                   architecture_attributes_,
                                   this,
                                   plugin_);
 }
 
 }  // namespace plugin