Pepper: Clean up trusted plugin arguments.

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

Index: ppapi/native_client/src/trusted/plugin/plugin.cc
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index 701851d2708060aab71eac70d5b01b7c5839890c..c19eaa16f764e0b06c7ca4e52c95559a4aba297e 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -119,24 +119,11 @@ bool Plugin::EarlyInit(int argc, const char* argn[], const char* argv[]) {
   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();
@@ -498,13 +485,10 @@ NaClSubprocess* Plugin::LoadHelperNaClModule(nacl::DescWrapper* wrapper,
   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()

 }
 
@@ -556,17 +540,13 @@ bool Plugin::Init(uint32_t argc, const char* argn[], const char* 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);
-
-    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);
-    }
+    enable_dev_interfaces_ = args_.find(kDevAttribute) != args_.end();
+
+    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.
@@ -574,6 +554,8 @@ bool Plugin::Init(uint32_t argc, const char* argn[], const char* argv[]) {
       // 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
@@ -585,12 +567,12 @@ bool Plugin::Init(uint32_t argc, const char* argn[], const char* argv[]) {
       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"));
       }
@@ -598,7 +580,7 @@ bool Plugin::Init(uint32_t argc, const char* argn[], const char* argv[]) {
       // 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());
     }
   }
 
@@ -609,9 +591,6 @@ bool Plugin::Init(uint32_t argc, const char* argn[], const char* argv[]) {
 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),
@@ -691,8 +670,6 @@ Plugin::~Plugin() {
   ShutDownSubprocesses();
 
   delete wrapper_factory_;
-  delete[] argv_;
-  delete[] argn_;
 
   HistogramTimeSmall(
       "NaCl.Perf.ShutdownTime.Total",