PPAPI: Move some of NexeFileDidOpen to NexeLoadManager

This also tickled a bug in the generator because I named a variable
with "http". Which made the generator tests run on presubmit. Which
tickled a bug in the tests :-). So those are fixed here as an aside.

BUG=239656

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

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

components/nacl/renderer/nexe_load_manager.cc

@@ -40,6 +40,24 @@
 
 namespace {
 
+void HistogramCustomCounts(const std::string& name,
+                           int32_t sample,
+                           int32_t min,
+                           int32_t max,
+                           uint32_t bucket_count) {
+  base::HistogramBase* counter =
+      base::Histogram::FactoryGet(
+          name,
+          min,
+          max,
+          bucket_count,
+          base::HistogramBase::kUmaTargetedHistogramFlag);
+  // The histogram can be NULL if it is constructed with bad arguments.  Ignore
+  // that data for this API.  An error message will be logged.
+  if (counter)
+    counter->Add(sample);
+}
+
 void HistogramEnumerate(const std::string& name,
                         int32_t sample,
                         int32_t boundary_value) {
@@ -135,6 +153,27 @@ void HistogramStartupTimeMedium(const std::string& name,
   }
 }
 
+void HistogramSizeKB(const std::string& name, int32_t sample) {
+  if (sample < 0) return;
+  HistogramCustomCounts(name,
+                        sample,
+                        1,
+                        512 * 1024,  // A very large .nexe.
+                        100);
+}
+
+void HistogramHTTPStatusCode(const std::string& name,
+                             int32_t status) {
+  // Log the status codes in rough buckets - 1XX, 2XX, etc.
+  int sample = status / 100;
+  // HTTP status codes only go up to 5XX, using "6" to indicate an internal
+  // error.
+  // Note: installed files may have "0" for a status code.
+  if (status < 0 || status >= 600)
+    sample = 6;
+  HistogramEnumerate(name, sample, 7);
+}
+
 blink::WebString EventTypeToString(PP_NaClEventType event_type) {
   switch (event_type) {
     case PP_NACL_EVENT_LOADSTART:
@@ -191,6 +230,53 @@ NexeLoadManager::~NexeLoadManager() {
   }
 }
 
+void NexeLoadManager::NexeFileDidOpen(int32_t pp_error,
+                                      int32_t fd,
+                                      int32_t http_status,
+                                      int64_t nexe_bytes_read,
+                                      const std::string& url) {
+  // Check that we are on the main renderer thread.
+  DCHECK(content::RenderThread::Get());
+  VLOG(1) << "Plugin::NexeFileDidOpen (pp_error=" << pp_error << ")";
+  VLOG(1) << "Plugin::NexeFileDidOpen (file_desc=" << fd << ")";
+  HistogramHTTPStatusCode(
+      is_installed_ ? "NaCl.HttpStatusCodeClass.Nexe.InstalledApp" :
+                      "NaCl.HttpStatusCodeClass.Nexe.NotInstalledApp",
+      http_status);
+  // TODO(dmichael): fd is only used for error reporting here currently, and
+  // the trusted Plugin is responsible for using it and closing it.
+  // Note -1 is NACL_NO_FILE_DESC from nacl_macros.h.
+  if (pp_error != PP_OK || fd == -1) {
+    if (pp_error == PP_ERROR_ABORTED) {
+      ReportLoadAbort();
+    } else if (pp_error == PP_ERROR_NOACCESS) {
+      ReportLoadError(PP_NACL_ERROR_NEXE_NOACCESS_URL,
+                      "access to nexe url was denied.");
+    } else {
+      ReportLoadError(PP_NACL_ERROR_NEXE_LOAD_URL,
+                      "could not load nexe url.");
+    }
+    return;
+  } else if (nexe_bytes_read == -1) {
+    ReportLoadError(PP_NACL_ERROR_NEXE_STAT, "could not stat nexe file.");
+    return;
+  }
+
+  // TODO(dmichael): Can we avoid stashing away so much state?
+  nexe_size_ = nexe_bytes_read;
+  HistogramSizeKB("NaCl.Perf.Size.Nexe",
+                  static_cast<int32_t>(nexe_size_ / 1024));
+
+  // Inform JavaScript that we successfully downloaded the nacl module.
+  ProgressEvent progress_event(pp_instance_, PP_NACL_EVENT_PROGRESS, url, true,
+      nexe_size_, nexe_size_);
+  ppapi::PpapiGlobals::Get()->GetMainThreadMessageLoop()->PostTask(
+      FROM_HERE,
+      base::Bind(&NexeLoadManager::DispatchEvent,
+                 weak_factory_.GetWeakPtr(),
+                 progress_event));
+}
+
 void NexeLoadManager::ReportLoadSuccess(const std::string& url,
                                         uint64_t loaded_bytes,
                                         uint64_t total_bytes) {

components/nacl/renderer/nexe_load_manager.h

@@ -28,6 +28,11 @@ class NexeLoadManager {
   explicit NexeLoadManager(PP_Instance instance);
   ~NexeLoadManager();
 
+  void NexeFileDidOpen(int32_t pp_error,
+                       int32_t fd,
+                       int32_t http_status,
+                       int64_t nexe_bytes_read,
+                       const std::string& url);
   void ReportLoadSuccess(const std::string& url,
                          uint64_t loaded_bytes,
                          uint64_t total_bytes);
@@ -91,7 +96,6 @@ class NexeLoadManager {
   void ReportStartupOverhead() const;
 
   int64_t nexe_size() const { return nexe_size_; }
-  void set_nexe_size(int64_t nexe_size) { nexe_size_ = nexe_size; }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NexeLoadManager);

components/nacl/renderer/ppb_nacl_private_impl.cc

@@ -476,6 +476,22 @@ void DispatchEventOnMainThread(PP_Instance instance,
   }
 }
 
+void NexeFileDidOpen(PP_Instance instance,
+                     int32_t pp_error,
+                     int32_t fd,
+                     int32_t http_status,
+                     int64_t nexe_bytes_read,
+                     const char* url) {
+  nacl::NexeLoadManager* load_manager = GetNexeLoadManager(instance);
+  if (load_manager) {
+    load_manager->NexeFileDidOpen(pp_error,
+                                  fd,
+                                  http_status,
+                                  nexe_bytes_read,
+                                  url);
+  }
+}
+
 void ReportLoadSuccess(PP_Instance instance,
                        const char* url,
                        uint64_t loaded_bytes,
@@ -517,6 +533,12 @@ void InstanceCreated(PP_Instance instance) {
 void InstanceDestroyed(PP_Instance instance) {
   NexeLoadManagerMap& map = g_load_manager_map.Get();
   DLOG_IF(ERROR, map.count(instance) == 0) << "Could not find instance ID";
+  // The erase may call NexeLoadManager's destructor prior to removing it from
+  // the map. In that case, it is possible for the trusted Plugin to re-enter
+  // the NexeLoadManager (e.g., by calling ReportLoadError). Passing out the
+  // NexeLoadManager to a local scoped_ptr just ensures that its entry is gone
+  // from the map prior to the destructor being invoked.
+  scoped_ptr<nacl::NexeLoadManager> temp(map.take(instance));
   map.erase(instance);
 }
 
@@ -629,13 +651,6 @@ int64_t GetNexeSize(PP_Instance instance) {
   return 0;
 }
 
-void SetNexeSize(PP_Instance instance, int64_t nexe_size) {
-  nacl::NexeLoadManager* load_manager = GetNexeLoadManager(instance);
-  DCHECK(load_manager);
-  if (load_manager)
-    return load_manager->set_nexe_size(nexe_size);
-}
-
 const PPB_NaCl_Private nacl_interface = {
   &LaunchSelLdr,
   &StartPpapiProxy,
@@ -650,6 +665,7 @@ const PPB_NaCl_Private nacl_interface = {
   &ReportTranslationFinished,
   &OpenNaClExecutable,
   &DispatchEvent,
+  &NexeFileDidOpen,
   &ReportLoadSuccess,
   &ReportLoadError,
   &ReportLoadAbort,
@@ -669,8 +685,7 @@ const PPB_NaCl_Private nacl_interface = {
   &SetExitStatus,
   &Vlog,
   &SetInitTime,
-  &GetNexeSize,
-  &SetNexeSize
+  &GetNexeSize
 };
 
 }  // namespace

ppapi/api/private/ppb_nacl_private.idl

@@ -271,6 +271,18 @@ interface PPB_NaCl_Private {
                      [in] uint64_t loaded_bytes,
                      [in] uint64_t total_bytes);
 
+  /* Report that the attempt to open the nexe has finished. Opening the file
+   * may have failed, as indicated by a pp_error value that is not PP_OK or an
+   * fd of -1. Failure to stat the file to determine its length results in
+   * nexe_bytes_read being -1.
+   */
+  void NexeFileDidOpen([in] PP_Instance instance,
+                       [in] int32_t pp_error,
+                       [in] int32_t fd,
+                       [in] int32_t http_status,
+                       [in] int64_t nexe_bytes_read,
+                       [in] str_t url);
+
   /* Report that the nexe loaded successfully. */
   void ReportLoadSuccess([in] PP_Instance instance,
                          [in] str_t url,

ppapi/c/private/ppb_nacl_private.h

@@ -3,7 +3,7 @@
  * found in the LICENSE file.
  */
 
-/* From private/ppb_nacl_private.idl modified Mon Apr  7 13:43:24 2014. */
+/* From private/ppb_nacl_private.idl modified Wed Apr  9 10:26:49 2014. */
 
 #ifndef PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
 #define PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
@@ -278,6 +278,17 @@ struct PPB_NaCl_Private_1_0 {
                         PP_Bool length_is_computable,
                         uint64_t loaded_bytes,
                         uint64_t total_bytes);
+  /* Report that the attempt to open the nexe has finished. Opening the file
+   * may have failed, as indicated by a pp_error value that is not PP_OK or an
+   * fd of -1. Failure to stat the file to determine its length results in
+   * nexe_bytes_read being -1.
+   */
+  void (*NexeFileDidOpen)(PP_Instance instance,
+                          int32_t pp_error,
+                          int32_t fd,
+                          int32_t http_status,
+                          int64_t nexe_bytes_read,
+                          const char* url);
   /* Report that the nexe loaded successfully. */
   void (*ReportLoadSuccess)(PP_Instance instance,
                             const char* url,

ppapi/generators/idl_c_proto.py

@@ -642,7 +642,7 @@ class CGen(object):
       # Add indentation
       line = tab + line
       space_break = line.rfind(' ', 0, 80)
-      if len(line) <= 80 or 'http' in line:
+      if len(line) <= 80 or 'http://' in line:
         # Ignore normal line and URLs permitted by the style guide.
         lines.append(line.rstrip())
       elif not '(' in line and space_break >= 0:

ppapi/generators/test_cgen_range/dev_channel_interface.h

@@ -11,7 +11,6 @@
 #define PPAPI_C_TEST_CGEN_RANGE_DEV_CHANNEL_INTERFACE_H_
 
 #include "ppapi/c/pp_macros.h"
-#include "ppapi/c/test_cgen_range/versions.h"
 
 #define TESTDEV_INTERFACE_1_0 "TestDev;1.0"
 #define TESTDEV_INTERFACE_1_2 "TestDev;1.2"

ppapi/generators/test_cgen_range/versions.h

@@ -9,6 +9,7 @@
 #define PPAPI_C_TEST_CGEN_RANGE_VERSIONS_H_
 
 #include "ppapi/c/pp_macros.h"
+#include "ppapi/c/test_cgen_range/dev_channel_interface.h"
 
 #define FOO_INTERFACE_0_0 "Foo;0.0"
 #define FOO_INTERFACE_1_0 "Foo;1.0"

ppapi/generators/test_cgen_range/versions.idl

@@ -13,10 +13,6 @@ label Chrome {
   M17 = 4.0
 };
 
-describe {
-  int32_t;
-};
-
 /* Bogus Interface Foo */
 [version=0.0]
 interface Foo {

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

@@ -191,8 +191,7 @@ void Plugin::HistogramEnumerateManifestIsDataURI(bool is_data_uri) {
   HistogramEnumerate("NaCl.Manifest.IsDataURI", is_data_uri, 2, -1);
 }
 
-void Plugin::HistogramHTTPStatusCode(const std::string& name,
-                                     int status) {
+void Plugin::HistogramHTTPStatusCode(const std::string& name, int status) {
   // Log the status codes in rough buckets - 1XX, 2XX, etc.
   int sample = status / 100;
   // HTTP status codes only go up to 5XX, using "6" to indicate an internal
@@ -614,54 +613,26 @@ void Plugin::HistogramStartupTimeMedium(const std::string& name, float dt) {
 }
 
 void Plugin::NexeFileDidOpen(int32_t pp_error) {
-  PLUGIN_PRINTF(("Plugin::NexeFileDidOpen (pp_error=%" NACL_PRId32 ")\n",
-                 pp_error));
   NaClFileInfo tmp_info(nexe_downloader_.GetFileInfo());
   NaClFileInfoAutoCloser info(&tmp_info);
-  PLUGIN_PRINTF(("Plugin::NexeFileDidOpen (file_desc=%" NACL_PRId32 ")\n",
-                 info.get_desc()));
-  HistogramHTTPStatusCode(
-      nacl_interface_->GetIsInstalled(pp_instance()) ?
-          "NaCl.HttpStatusCodeClass.Nexe.InstalledApp" :
-          "NaCl.HttpStatusCodeClass.Nexe.NotInstalledApp",
-      nexe_downloader_.status_code());
-  ErrorInfo error_info;
-  if (pp_error != PP_OK || info.get_desc() == NACL_NO_FILE_DESC) {
-    if (pp_error == PP_ERROR_ABORTED) {
-      ReportLoadAbort();
-    } else if (pp_error == PP_ERROR_NOACCESS) {
-      error_info.SetReport(PP_NACL_ERROR_NEXE_NOACCESS_URL,
-                           "access to nexe url was denied.");
-      ReportLoadError(error_info);
-    } else {
-      error_info.SetReport(PP_NACL_ERROR_NEXE_LOAD_URL,
-                           "could not load nexe url.");
-      ReportLoadError(error_info);
-    }
-    return;
-  }
-  struct stat stat_buf;
-  if (0 != fstat(info.get_desc(), &stat_buf)) {
-    error_info.SetReport(PP_NACL_ERROR_NEXE_STAT, "could not stat nexe file.");
-    ReportLoadError(error_info);
-    return;
+
+  int64_t nexe_bytes_read = -1;
+  if (pp_error == PP_OK && info.get_desc() != NACL_NO_FILE_DESC) {
+    struct stat stat_buf;
+    if (0 == fstat(info.get_desc(), &stat_buf))
+      nexe_bytes_read = stat_buf.st_size;
   }
-  size_t nexe_bytes_read = static_cast<size_t>(stat_buf.st_size);
 
-  nacl_interface_->SetNexeSize(pp_instance(), nexe_bytes_read);
-  HistogramSizeKB("NaCl.Perf.Size.Nexe",
-                  static_cast<int32_t>(nexe_bytes_read / 1024));
+  nacl_interface_->NexeFileDidOpen(pp_instance(),
+                                   pp_error,
+                                   info.get_desc(),
+                                   nexe_downloader_.status_code(),
+                                   nexe_bytes_read,
+                                   nexe_downloader_.url().c_str());
   HistogramStartupTimeMedium(
       "NaCl.Perf.StartupTime.NexeDownload",
       static_cast<float>(nexe_downloader_.TimeSinceOpenMilliseconds()));
 
-  // Inform JavaScript that we successfully downloaded the nacl module.
-  EnqueueProgressEvent(PP_NACL_EVENT_PROGRESS,
-                       nexe_downloader_.url(),
-                       LENGTH_IS_COMPUTABLE,
-                       nexe_bytes_read,
-                       nexe_bytes_read);
-
   load_start_ = NaClGetTimeOfDayMicroseconds();
   nacl::scoped_ptr<nacl::DescWrapper>
       wrapper(wrapper_factory()->MakeFileDesc(info.Release().desc, O_RDONLY));

ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c

@@ -3152,6 +3152,11 @@ static void Pnacl_M25_PPB_NaCl_Private_DispatchEvent(PP_Instance instance, PP_Na
   iface->DispatchEvent(instance, event_type, resource_url, length_is_computable, loaded_bytes, total_bytes);
 }
 
+static void Pnacl_M25_PPB_NaCl_Private_NexeFileDidOpen(PP_Instance instance, int32_t pp_error, int32_t fd, int32_t http_status, int64_t nexe_bytes_read, const char* url) {
+  const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface;
+  iface->NexeFileDidOpen(instance, pp_error, fd, http_status, nexe_bytes_read, url);
+}
+
 static void Pnacl_M25_PPB_NaCl_Private_ReportLoadSuccess(PP_Instance instance, const char* url, uint64_t loaded_bytes, uint64_t total_bytes) {
   const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface;
   iface->ReportLoadSuccess(instance, url, loaded_bytes, total_bytes);
@@ -5144,6 +5149,7 @@ static const struct PPB_NaCl_Private_1_0 Pnacl_Wrappers_PPB_NaCl_Private_1_0 = {
     .ReportTranslationFinished = (void (*)(PP_Instance instance, PP_Bool success))&Pnacl_M25_PPB_NaCl_Private_ReportTranslationFinished,
     .OpenNaClExecutable = (PP_FileHandle (*)(PP_Instance instance, const char* file_url, uint64_t* file_token_lo, uint64_t* file_token_hi))&Pnacl_M25_PPB_NaCl_Private_OpenNaClExecutable,
     .DispatchEvent = (void (*)(PP_Instance instance, PP_NaClEventType event_type, const char* resource_url, PP_Bool length_is_computable, uint64_t loaded_bytes, uint64_t total_bytes))&Pnacl_M25_PPB_NaCl_Private_DispatchEvent,
+    .NexeFileDidOpen = (void (*)(PP_Instance instance, int32_t pp_error, int32_t fd, int32_t http_status, int64_t nexe_bytes_read, const char* url))&Pnacl_M25_PPB_NaCl_Private_NexeFileDidOpen,
     .ReportLoadSuccess = (void (*)(PP_Instance instance, const char* url, uint64_t loaded_bytes, uint64_t total_bytes))&Pnacl_M25_PPB_NaCl_Private_ReportLoadSuccess,
     .ReportLoadError = (void (*)(PP_Instance instance, PP_NaClError error, const char* error_message, const char* console_message))&Pnacl_M25_PPB_NaCl_Private_ReportLoadError,
     .ReportLoadAbort = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_ReportLoadAbort,