It looks like the Chrome NPAPI plugin installer has been broken since we first updated chrome webkit after 1.0 shipped.

Basically in the 1.0 branch when the plugin was instantiated in its instantiation it would get the mime type along with the list of other arguments. If an object tag was specified with the classid, it would get mapped to the mime type. With the webkit merge the classs id is passed in along with the mime type.

The plugin installer thinks that this is an activex installation on receiving a valid class id and 
and ends up checking if it is a white listed classid, etc.

All this code will be taken out along with the activex shim in the near future. For now we take this code path only if we don't have a valid mime type.

This fixes http://code.google.com/p/chromium/issues/detail?id=8584

Added a plugin test for the argument parsing functionality in the default plugin. I changed the ParseInstantiationArguments function in the
plugin installer to a static function to be able to unit test this.


Bug=8584

Review URL: http://codereview.chromium.org/42684

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

2 files changed, 93 insertions, 52 deletions

diff --git a/webkit/default_plugin/plugin_impl_win.cc b/webkit/default_plugin/plugin_impl_win.cc
index 30c166f..70f9cfe 100644
--- a/webkit/default_plugin/plugin_impl_win.cc
+++ b/webkit/default_plugin/plugin_impl_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2009 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.
 
@@ -65,10 +65,17 @@ bool PluginInstallerImpl::Initialize(HINSTANCE module_handle, NPP instance,
   DCHECK(instance != NULL);
   DCHECK(module_handle != NULL);
 
+  instance_ = instance;
+  mime_type_ = mime_type;
+
   // The clsid without the {} parentheses.
   std::string raw_activex_clsid;
   if (!ParseInstantiationArguments(mime_type, instance, argc, argn, argv,
-                                   &raw_activex_clsid)) {
+                                   &raw_activex_clsid, &is_activex_,
+                                   &activex_clsid_,
+                                   &activex_codebase_,
+                                   &plugin_download_url_,
+                                   &plugin_finder_url_)) {
     DLOG(ERROR) << "Incorrect arguments passed to plugin";
     NOTREACHED();
     return false;
@@ -667,13 +674,18 @@ LRESULT PluginInstallerImpl::OnActiveXInstallResult(UINT message,
 
 std::string PluginInstallerImpl::ResolveURL(NPP instance,
                                             const std::string& relative_url) {
-  NPObject* object = NULL;
-  default_plugin::g_browser->getvalue(instance, NPNVWindowNPObject, &object);
-  activex_shim::NPNScriptableObject window(instance, object);
-  std::wstring url =
-      window.GetObjectProperty("document").GetStringProperty("URL");
-  GURL base(url);
-  return base.Resolve(relative_url).spec();
+  // The NPAPI functions may not be available if this function is called
+  // as a result of a unit test.
+  if (default_plugin::g_browser) {
+    NPObject* object = NULL;
+    default_plugin::g_browser->getvalue(instance, NPNVWindowNPObject, &object);
+    activex_shim::NPNScriptableObject window(instance, object);
+    std::wstring url =
+        window.GetObjectProperty("document").GetStringProperty("URL");
+    GURL base(url);
+    return base.Resolve(relative_url).spec();
+  }
+  return relative_url;
 }
 
 bool PluginInstallerImpl::InitializeResources(HINSTANCE module_handle) {
@@ -702,42 +714,62 @@ bool PluginInstallerImpl::InitializeResources(HINSTANCE module_handle) {
 }
 
 bool PluginInstallerImpl::ParseInstantiationArguments(
-    NPMIMEType mime_type, NPP instance, int16 argc, char* argn[], char* argv[],
-    std::string* raw_activex_clsid) {
-  instance_ = instance;
-  mime_type_ = mime_type;
+    NPMIMEType mime_type,
+    NPP instance,
+    int16 argc,
+    char* argn[],
+    char* argv[],
+    std::string* raw_activex_clsid,
+    bool* is_activex,
+    std::string* activex_clsid,
+    std::string* activex_codebase,
+    std::string* plugin_download_url,
+    std::string* plugin_finder_url) {
+
+  if (!raw_activex_clsid || !is_activex || !activex_clsid ||
+      !plugin_download_url || !plugin_finder_url || !activex_codebase) {
+    NOTREACHED();
+    return false;
+  }
+
+  *is_activex = false;
+
+  bool valid_mime_type = (mime_type != NULL ? strlen(mime_type) > 0 : false);
 
   for (int i = 0; i < argc; ++i) {
-    if (LowerCaseEqualsASCII(argn[i], "classid") &&
+    // We should only look for activex installation if the mime type passed in
+    // is not valid. In any case this code will be taken out when we remove
+    // the activex shim.
+    if (!valid_mime_type && LowerCaseEqualsASCII(argn[i], "classid") &&
         activex_shim::GetClsidFromClassidAttribute(argv[i],
                                                    raw_activex_clsid)) {
-      is_activex_ = true;
-      activex_clsid_ = std::string("{") + *raw_activex_clsid + "}";
+      *is_activex = true;
+      *activex_clsid = std::string("{") + *raw_activex_clsid + "}";
     }
     if (LowerCaseEqualsASCII(argn[i], "codebase")) {
-      activex_codebase_ = ResolveURL(instance, argv[i]);
-      size_t pos = activex_codebase_.find('#');
+      *activex_codebase = ResolveURL(instance, argv[i]);
+      size_t pos = activex_codebase->find('#');
       if (pos != std::string::npos)
-        plugin_download_url_ = activex_codebase_.substr(0, pos);
+        *plugin_download_url = activex_codebase->substr(0, pos);
       else
-        plugin_download_url_ = activex_codebase_;
+        *plugin_download_url = *activex_codebase;
     }
   }
 
-  if (!is_activex_) {
-    if (!mime_type || !instance) {
+  if (!*is_activex) {
+    if (!valid_mime_type || !instance) {
       DLOG(WARNING) << __FUNCTION__ << " Invalid parameters passed in";
       NOTREACHED();
       return false;
     }
 
-    if (!webkit_glue::GetPluginFinderURL(&plugin_finder_url_)) {
+    if (!webkit_glue::GetPluginFinderURL(plugin_finder_url)) {
       NOTREACHED();
       DLOG(WARNING) << __FUNCTION__ << " Failed to get the plugin finder URL";
       return false;
     }
 
-    DLOG(INFO) << "Plugin finder URL is " << plugin_finder_url_.c_str();
+    DLOG(INFO) << "Plugin finder URL is " << plugin_finder_url->c_str();
   }
 
   return true;
diff --git a/webkit/default_plugin/plugin_impl_win.h b/webkit/default_plugin/plugin_impl_win.h
index 8737ce5..66a97c6 100644
--- a/webkit/default_plugin/plugin_impl_win.h
+++ b/webkit/default_plugin/plugin_impl_win.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2009 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.
 
@@ -191,6 +191,41 @@ class PluginInstallerImpl : public CWindowImpl<PluginInstallerImpl> {
   // Arabic).
   bool IsRTLLayout() const;
 
+  // Parses the plugin instantiation arguments. This includes checking for
+  // whether this is an activex install and reading the appropriate
+  // arguments like codebase, etc. For plugin installs we download the
+  // plugin finder URL and initalize the mime type and the plugin instance
+  // info.
+  //
+  // Parameters:
+  // module_handle: The handle to the dll in which this object is instantiated.
+  // instance: The plugins opaque instance handle.
+  // mime_type: Identifies the third party plugin
+  // argc: Indicates the count of arguments passed in from the webpage.
+  // argv: Pointer to the arguments.
+  // raw_activex_clsid: Output parameter which contains the CLSID of the
+  //                    Activex plugin needed for an Activex install
+  // is_activex: Output parameter indicating if this is an activex install
+  // activex_clsid: Output parameter containing the classid of the activex
+  //                for an activex install
+  // activex_codebase: Output parameter containing the activex codebase if
+  //                   this is an activex install
+  // plugin_download_url: Output parameter containing the plugin download url
+  //                      on success.
+  // plugin_finder_url: Output parameter containing the plugin finder url on
+  //                    success.
+  // Returns true on success.
+  static bool ParseInstantiationArguments(NPMIMEType mime_type,
+                                          NPP instance,
+                                          int16 argc,
+                                          char* argn[],
+                                          char* argv[],
+                                          std::string* raw_activex_clsid,
+                                          bool* is_activex,
+                                          std::string* activex_clsid,
+                                          std::string* activex_codebase,
+                                          std::string* plugin_download_url,
+                                          std::string* plugin_finder_url);
  protected:
   // Window message handlers.
   LRESULT OnPaint(UINT message, WPARAM wparam, LPARAM lparam, BOOL& handled);
@@ -254,7 +289,7 @@ class PluginInstallerImpl : public CWindowImpl<PluginInstallerImpl> {
   // relative_url
   //   The URL to be resolved.
   // Returns the resolved URL.
-  std::string ResolveURL(NPP instance, const std::string& relative_url);
+  static std::string ResolveURL(NPP instance, const std::string& relative_url);
 
   // Initializes resources like the icon, fonts, etc needed by the plugin
   // installer
@@ -265,32 +300,6 @@ class PluginInstallerImpl : public CWindowImpl<PluginInstallerImpl> {
   // Returns true on success.
   bool InitializeResources(HINSTANCE module_handle);
 
-  // Parses the plugin instantiation arguments. This includes checking for
-  // whether this is an activex install and reading the appropriate
-  // arguments like codebase, etc. For plugin installs we download the
-  // plugin finder URL and initalize the mime type and the plugin instance
-  // info.
-  //
-  // Parameters:
-  // module_handle
-  //   The handle to the dll in which this object is instantiated.
-  // instance
-  //   The plugins opaque instance handle.
-  // mime_type
-  //   Identifies the third party plugin which would be eventually installed.
-  // argc
-  //   Indicates the count of arguments passed in from the webpage.
-  // argv
-  //   Pointer to the arguments.
-  // raw_activex_clsid
-  //   Output parameter which contains the CLSID of the Activex plugin needed.
-  //   This is only applicable if the webpage specifically requests an ActiveX
-  //   control.
-  // Returns true on success.
-  bool ParseInstantiationArguments(NPMIMEType mime_type, NPP instance,
-                                   int16 argc, char* argn[], char* argv[],
-                                   std::string* raw_activex_clsid);
-
   // Paints user action messages to the plugin window. These include messages
   // like whether the user should click on the plugin window to download the
   // plugin, etc.