Pepper: Clean up trusted plugin arguments.

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 101 matching lines @@
 #ifdef NACL_OSX
   // TODO(kochi): For crbug.com/102808, this is a stopgap solution for Lion
   // until we expose IME API to .nexe. This disables any IME interference
   // against key inputs, so you cannot use off-the-spot IME input for NaCl apps.
   // This makes discrepancy among platforms and therefore we should remove
   // this hack when IME API is made available.
   // The default for non-Mac platforms is still off-the-spot IME mode.
   pp::TextInputController(this).SetTextInputType(PP_TEXTINPUT_TYPE_NONE);
 #endif
 
-  // Remember the embed/object argn/argv pairs.
-  argn_ = new char*[argc];
-  argv_ = new char*[argc];
-  argc_ = 0;
   for (int i = 0; i < argc; ++i) {
-    if (NULL != argn_ && NULL != argv_) {
-      argn_[argc_] = strdup(argn[i]);
-      argv_[argc_] = strdup(argv[i]);
-      if (NULL == argn_[argc_] || NULL == argv_[argc_]) {
-        // Give up on passing arguments.
-        free(argn_[argc_]);
-        free(argv_[argc_]);
-        continue;
-      }
-      ++argc_;
-    }
+    std::string name(argn[i]);
+    std::string value(argv[i]);

[dmichael (off chromium), 2014/02/12 22:06:46]

nit: arguably, you could not have these temp variables at all. Or use const
std::string&. But it's not important.

+    args_[name] = value;
   }
-  // TODO(sehr): this leaks strings if there is a subsequent failure.
 
   // Set up the factory used to produce DescWrappers.
   wrapper_factory_ = new nacl::DescWrapperFactory();
   if (NULL == wrapper_factory_) {
     return false;
   }
   PLUGIN_PRINTF(("Plugin::Init (wrapper_factory=%p)\n",
                  static_cast<void*>(wrapper_factory_)));
 
   PLUGIN_PRINTF(("Plugin::Init (return 1)\n"));
@@ skipping 341 matching lines @@
                           nacl_subprocess->description());
     return NULL;
   }
 
   PLUGIN_PRINTF(("Plugin::LoadHelperNaClModule (%s)\n",
                  nacl_subprocess.get()->detailed_description().c_str()));
 
   return nacl_subprocess.release();
 }
 
-char* Plugin::LookupArgument(const char* key) {
-  char** keys = argn_;
-  for (int ii = 0, len = argc_; ii < len; ++ii) {
-    if (!strcmp(keys[ii], key)) {
-      return argv_[ii];
-    }
-  }
+std::string Plugin::LookupArgument(const std::string& key) const {
+  std::map<std::string, std::string>::const_iterator it = args_.find(key);
+  if (it != args_.end())
+    return it->second;
   return NULL;

[dmichael (off chromium), 2014/02/12 22:06:46]

I think it would be clearer to return std::string()

 }
 
 const char* const Plugin::kNaClMIMEType = "application/x-nacl";
 const char* const Plugin::kPnaclMIMEType = "application/x-pnacl";
 
 bool Plugin::NexeIsContentHandler() const {
   // Tests if the MIME type is not a NaCl MIME type.
   // If the MIME type is foreign, then this NEXE is being used as a content
   // type handler rather than directly by an HTML document.
   return
@@ skipping 30 matching lines @@
   if (url_util_ == NULL)
     return false;
 
   PLUGIN_PRINTF(("Plugin::Init (url_util_=%p)\n",
                  static_cast<const void*>(url_util_)));
 
   bool status = EarlyInit(static_cast<int>(argc), argn, argv);
   if (status) {
     // Look for the developer attribute; if it's present, enable 'dev'
     // interfaces.
-    const char* dev_settings = LookupArgument(kDevAttribute);
-    enable_dev_interfaces_ = (dev_settings != NULL);
+    enable_dev_interfaces_ = args_.find(kDevAttribute) != args_.end();
 
-    const char* type_attr = LookupArgument(kTypeAttribute);
-    if (type_attr != NULL) {
-      mime_type_ = nacl::string(type_attr);
-      std::transform(mime_type_.begin(), mime_type_.end(), mime_type_.begin(),
-                     tolower);
-    }
+    mime_type_ = LookupArgument(kTypeAttribute);
+    std::transform(mime_type_.begin(), mime_type_.end(), mime_type_.begin(),
+                   tolower);
 
-    const char* manifest_url = LookupArgument(kSrcManifestAttribute);
+    std::string manifest_url;
     if (NexeIsContentHandler()) {
       // For content handlers 'src' will be the URL for the content
       // and 'nacl' will be the URL for the manifest.
       manifest_url = LookupArgument(kNaClManifestAttribute);
       // For content handlers the NEXE runs in the security context of the
       // content it is rendering and the NEXE itself appears to be a
       // cross-origin resource stored in a Chrome extension.
+    } else {
+      manifest_url = LookupArgument(kSrcManifestAttribute);
     }
     // Use the document URL as the base for resolving relative URLs to find the
     // manifest.  This takes into account the setting of <base> tags that
     // precede the embed/object.
     CHECK(url_util_ != NULL);
     pp::Var base_var = url_util_->GetDocumentURL(this);
     if (!base_var.is_string()) {
       PLUGIN_PRINTF(("Plugin::Init (unable to find document url)\n"));
       return false;
     }
     set_plugin_base_url(base_var.AsString());
-    if (manifest_url == NULL) {
+    if (manifest_url.empty()) {
       // TODO(sehr,polina): this should be a hard error when scripting
       // the src property is no longer allowed.
       PLUGIN_PRINTF(("Plugin::Init:"
                      " WARNING: no 'src' property, so no manifest loaded.\n"));
-      if (NULL != LookupArgument(kNaClManifestAttribute)) {
+      if (args_.find(kNaClManifestAttribute) != args_.end()) {
         PLUGIN_PRINTF(("Plugin::Init:"
                        " WARNING: 'nacl' property is incorrect. Use 'src'.\n"));
       }
     } else {
       // Issue a GET for the manifest_url.  The manifest file will be parsed to
       // determine the nexe URL.
       // Sets src property to full manifest URL.
-      RequestNaClManifest(manifest_url);
+      RequestNaClManifest(manifest_url.c_str());
     }
   }
 
   PLUGIN_PRINTF(("Plugin::Init (status=%d)\n", status));
   return status;
 }
 
 Plugin::Plugin(PP_Instance pp_instance)
     : pp::InstancePrivate(pp_instance),
       scriptable_plugin_(NULL),
-      argc_(-1),
-      argn_(NULL),
-      argv_(NULL),
       main_subprocess_("main subprocess", NULL, NULL),
       nexe_error_reported_(false),
       wrapper_factory_(NULL),
       enable_dev_interfaces_(false),
       is_installed_(false),
       init_time_(0),
       ready_time_(0),
       nexe_size_(0),
       time_of_last_progress_event_(0),
       exit_status_(-1),
@@ skipping 59 matching lines @@
   // threads.
   //
   // The main_subprocess object, which wraps the main service_runtime
   // object, is dtor'd implicitly after the explicit code below runs,
   // so the main service runtime object will not have been dtor'd,
   // though the Shutdown method may have been called, during the
   // lifetime of the service threads.
   ShutDownSubprocesses();
 
   delete wrapper_factory_;
-  delete[] argv_;
-  delete[] argn_;
 
   HistogramTimeSmall(
       "NaCl.Perf.ShutdownTime.Total",
       (NaClGetTimeOfDayMicroseconds() - shutdown_start)
           / NACL_MICROS_PER_MILLI);
 
   PLUGIN_PRINTF(("Plugin::~Plugin (this=%p, return)\n",
                  static_cast<void*>(this)));
 }
 
@@ skipping 823 matching lines @@
   DCHECK(pp::Module::Get()->core()->IsMainThread());
   DCHECK(nacl_interface_);
   exit_status_ = exit_status;
   nacl_interface_->SetReadOnlyProperty(pp_instance(),
                                        pp::Var("exitStatus").pp_var(),
                                        pp::Var(exit_status_).pp_var());
 }
 
 
 }  // namespace plugin