plugins: drop PluginVersionInfo for internal plugins

The PluginVersionInfo structure just mirrored the Windows file
metadata format.  Since we want to create a WebPluginInfo anyway,
just store a WebPluginInfo as the internal plugin info.

TEST=refactoring change, should be covered by existing tests

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

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

6 files changed, 106 insertions, 100 deletions

diff --git a/chrome/browser/chromeos/gview_request_interceptor_unittest.cc b/chrome/browser/chromeos/gview_request_interceptor_unittest.cc
index 63f76cc..556a9f7 100644
--- a/chrome/browser/chromeos/gview_request_interceptor_unittest.cc
+++ b/chrome/browser/chromeos/gview_request_interceptor_unittest.cc
@@ -66,7 +66,10 @@ class GViewRequestInterceptorTest : public testing::Test {
   }
 
   void RegisterPDFPlugin() {
-    webkit::npapi::PluginList::Singleton()->RegisterInternalPlugin(pdf_path_);
+    webkit::npapi::WebPluginInfo info;
+    info.path = pdf_path_;
+    info.enabled = true;
+    webkit::npapi::PluginList::Singleton()->RegisterInternalPlugin(info);
     webkit::npapi::PluginList::Singleton()->RefreshPlugins();
   }
 
diff --git a/chrome/browser/plugin_service.cc b/chrome/browser/plugin_service.cc
index e136f97c..fbb2fae 100644
--- a/chrome/browser/plugin_service.cc
+++ b/chrome/browser/plugin_service.cc
@@ -459,20 +459,25 @@ void PluginService::RegisterPepperPlugins() {
   std::vector<PepperPluginInfo> plugins;
   PepperPluginRegistry::GetList(&plugins);
   for (size_t i = 0; i < plugins.size(); ++i) {
-    webkit::npapi::PluginVersionInfo info;
+    webkit::npapi::WebPluginInfo info;
     info.path = plugins[i].path;
-    info.product_name = plugins[i].name.empty() ?
-        plugins[i].path.BaseName().ToWStringHack() :
-        ASCIIToWide(plugins[i].name);
-    info.file_description = ASCIIToWide(plugins[i].description);
-    info.file_extensions = ASCIIToWide(plugins[i].file_extensions);
-    info.file_description = ASCIIToWide(plugins[i].type_descriptions);
-    info.mime_types = ASCIIToWide(JoinString(plugins[i].mime_types, '|'));
-
-    // These NPAPI entry points will never be called.  TODO(darin): Come up
-    // with a cleaner way to register pepper plugins with the NPAPI PluginList,
-    // or perhaps refactor the PluginList to be less specific to NPAPI.
-    memset(&info.entry_points, 0, sizeof(info.entry_points));
+    info.name = plugins[i].name.empty() ?
+        WideToUTF16(plugins[i].path.BaseName().ToWStringHack()) :
+        ASCIIToUTF16(plugins[i].name);
+    info.desc = ASCIIToUTF16(plugins[i].description);
+    info.enabled = true;
+
+    // TODO(evan): Pepper shouldn't require us to parse strings to get
+    // the list of mime types out.
+    if (!webkit::npapi::PluginList::ParseMimeTypes(
+            JoinString(plugins[i].mime_types, '|'),
+            plugins[i].file_extensions,
+            ASCIIToUTF16(plugins[i].type_descriptions),
+            &info.mime_types)) {
+      LOG(ERROR) << "Error parsing mime types for "
+                 << plugins[i].path.ToWStringHack();
+      return;
+    }
 
     webkit::npapi::PluginList::Singleton()->RegisterInternalPlugin(info);
   }
diff --git a/webkit/glue/plugins/plugin_list.h b/webkit/glue/plugins/plugin_list.h
index 111e8fa..580d8dd 100644
--- a/webkit/glue/plugins/plugin_list.h
+++ b/webkit/glue/plugins/plugin_list.h
@@ -18,7 +18,6 @@
 namespace NPAPI {
 
 typedef webkit::npapi::PluginEntryPoints PluginEntryPoints;
-typedef webkit::npapi::PluginVersionInfo PluginVersionInfo;
 
 class PluginList {
  public:
diff --git a/webkit/plugins/npapi/plugin_lib_win.cc b/webkit/plugins/npapi/plugin_lib_win.cc
index 29d014f..4b6e361 100644
--- a/webkit/plugins/npapi/plugin_lib_win.cc
+++ b/webkit/plugins/npapi/plugin_lib_win.cc
@@ -8,6 +8,7 @@
 #include "base/file_version_info_win.h"
 #include "base/logging.h"
 #include "base/path_service.h"
+#include "base/string_util.h"
 #include "webkit/plugins/npapi/plugin_constants_win.h"
 #include "webkit/plugins/npapi/plugin_list.h"
 
@@ -32,16 +33,25 @@ bool PluginLib::ReadWebPluginInfo(const FilePath &filename,
 
   FileVersionInfoWin* version_info_win =
       static_cast<FileVersionInfoWin*>(version_info.get());
-  PluginVersionInfo pvi;
-  pvi.mime_types = version_info_win->GetStringValue(L"MIMEType");
-  pvi.file_extensions = version_info_win->GetStringValue(L"FileExtents");
-  pvi.type_descriptions = version_info_win->GetStringValue(L"FileOpenName");
-  pvi.product_name = version_info->product_name();
-  pvi.file_description = version_info->file_description();
-  pvi.file_version = version_info->file_version();
-  pvi.path = filename;
-
-  return PluginList::CreateWebPluginInfo(pvi, info);
+
+  info->name = version_info->product_name();
+  info->desc = version_info->file_description();
+  info->version = version_info->file_version();
+  info->path = filename;
+  info->enabled = true;
+
+  // TODO(evan): Move the ParseMimeTypes code inline once Pepper is updated.
+  if (!PluginList::ParseMimeTypes(
+          UTF16ToASCII(version_info_win->GetStringValue(L"MIMEType")),
+          UTF16ToASCII(version_info_win->GetStringValue(L"FileExtents")),
+          version_info_win->GetStringValue(L"FileOpenName"),
+          &info->mime_types)) {
+    LOG_IF(ERROR, PluginList::DebugPluginLoading())
+        << "Plugin " << info->name << " has bad MIME types, skipping";
+    return false;
+  }
+
+  return true;
 }
 
 }  // namespace npapi
diff --git a/webkit/plugins/npapi/plugin_list.cc b/webkit/plugins/npapi/plugin_list.cc
index db82d35..b2e236a 100644
--- a/webkit/plugins/npapi/plugin_list.cc
+++ b/webkit/plugins/npapi/plugin_list.cc
@@ -216,40 +216,40 @@ void PluginList::AddExtraPluginDir(const FilePath& plugin_dir) {
 #endif
 }
 
-void PluginList::RegisterInternalPlugin(const PluginVersionInfo& info) {
-  AutoLock lock(lock_);
-  internal_plugins_.push_back(info);
-}
+void PluginList::RegisterInternalPlugin(const WebPluginInfo& info) {
+  PluginEntryPoints entry_points = {0};
+  InternalPlugin plugin = { info, entry_points };
 
-void PluginList::RegisterInternalPlugin(const FilePath& path) {
-  webkit::npapi::PluginVersionInfo info;
-  info.path = path;
-  memset(&info.entry_points, 0, sizeof(info.entry_points));
-  RegisterInternalPlugin(info);
+  AutoLock lock(lock_);
+  internal_plugins_.push_back(plugin);
 }
 
 void PluginList::RegisterInternalPlugin(const FilePath& filename,
                                         const std::string& name,
                                         const std::string& description,
-                                        const std::string& mime_type,
+                                        const std::string& mime_type_str,
                                         const PluginEntryPoints& entry_points) {
-  webkit::npapi::PluginVersionInfo info = {
-    filename,
-    ASCIIToWide(name),
-    ASCIIToWide(description),
-    L"1",
-    ASCIIToWide(mime_type),
-    L"",
-    L"",
-    entry_points
-  };
-  RegisterInternalPlugin(info);
+  InternalPlugin plugin;
+  plugin.info.path = filename;
+  plugin.info.name = ASCIIToUTF16(name);
+  plugin.info.version = ASCIIToUTF16("1");
+  plugin.info.desc = ASCIIToUTF16(description);
+  plugin.info.enabled = true;
+
+  WebPluginMimeType mime_type;
+  mime_type.mime_type = mime_type_str;
+  plugin.info.mime_types.push_back(mime_type);
+
+  plugin.entry_points = entry_points;
+
+  AutoLock lock(lock_);
+  internal_plugins_.push_back(plugin);
 }
 
 void PluginList::UnregisterInternalPlugin(const FilePath& path) {
   AutoLock lock(lock_);
   for (size_t i = 0; i < internal_plugins_.size(); i++) {
-    if (internal_plugins_[i].path == path) {
+    if (internal_plugins_[i].info.path == path) {
       internal_plugins_.erase(internal_plugins_.begin() + i);
       return;
     }
@@ -263,9 +263,10 @@ bool PluginList::ReadPluginInfo(const FilePath& filename,
   {
     AutoLock lock(lock_);
     for (size_t i = 0; i < internal_plugins_.size(); ++i) {
-      if (filename == internal_plugins_[i].path) {
+      if (filename == internal_plugins_[i].info.path) {
         *entry_points = &internal_plugins_[i].entry_points;
-        return CreateWebPluginInfo(internal_plugins_[i], info);
+        *info = internal_plugins_[i].info;
+        return true;
       }
     }
   }
@@ -276,27 +277,22 @@ bool PluginList::ReadPluginInfo(const FilePath& filename,
   return PluginLib::ReadWebPluginInfo(filename, info);
 }
 
-bool PluginList::CreateWebPluginInfo(const PluginVersionInfo& pvi,
-                                     WebPluginInfo* info) {
+// static
+bool PluginList::ParseMimeTypes(
+    const std::string& mime_types_str,
+    const std::string& file_extensions_str,
+    const string16& mime_type_descriptions_str,
+    std::vector<WebPluginMimeType>* parsed_mime_types) {
   std::vector<std::string> mime_types, file_extensions;
   std::vector<string16> descriptions;
-  base::SplitString(WideToUTF8(pvi.mime_types), '|', &mime_types);
-  base::SplitString(WideToUTF8(pvi.file_extensions), '|', &file_extensions);
-  base::SplitString(WideToUTF16(pvi.type_descriptions), '|', &descriptions);
+  base::SplitString(mime_types_str, '|', &mime_types);
+  base::SplitString(file_extensions_str, '|', &file_extensions);
+  base::SplitString(mime_type_descriptions_str, '|', &descriptions);
 
-  info->mime_types.clear();
+  parsed_mime_types->clear();
 
-  if (mime_types.empty()) {
-    LOG_IF(ERROR, PluginList::DebugPluginLoading())
-        << "Plugin " << pvi.product_name << " has no MIME types, skipping";
+  if (mime_types.empty())
     return false;
-  }
-
-  info->name = WideToUTF16(pvi.product_name);
-  info->desc = WideToUTF16(pvi.file_description);
-  info->version = WideToUTF16(pvi.file_version);
-  info->path = pvi.path;
-  info->enabled = true;
 
   for (size_t i = 0; i < mime_types.size(); ++i) {
     WebPluginMimeType mime_type;
@@ -312,14 +308,14 @@ bool PluginList::CreateWebPluginInfo(const PluginVersionInfo& pvi,
       // list from the description if it is present.
       size_t ext = mime_type.description.find(ASCIIToUTF16("(*"));
       if (ext != string16::npos) {
-        if (ext > 1 && mime_type.description[ext -1] == ' ')
+        if (ext > 1 && mime_type.description[ext - 1] == ' ')
           ext--;
 
         mime_type.description.erase(ext);
       }
     }
 
-    info->mime_types.push_back(mime_type);
+    parsed_mime_types->push_back(mime_type);
   }
 
   return true;
@@ -348,7 +344,7 @@ void PluginList::LoadPlugins(bool refresh) {
   // other methods if they're called on other threads.
   std::vector<FilePath> extra_plugin_paths;
   std::vector<FilePath> extra_plugin_dirs;
-  std::vector<PluginVersionInfo> internal_plugins;
+  std::vector<InternalPlugin> internal_plugins;
   {
     AutoLock lock(lock_);
     if (plugins_loaded_ && !refresh && !plugins_need_refresh_)
@@ -372,9 +368,9 @@ void PluginList::LoadPlugins(bool refresh) {
   // "discovered" plugin want to handle the same type, the internal plugin
   // will have precedence.
   for (size_t i = 0; i < internal_plugins.size(); ++i) {
-    if (internal_plugins[i].path.value() == kDefaultPluginLibraryName)
+    if (internal_plugins[i].info.path.value() == kDefaultPluginLibraryName)
       continue;
-    LoadPlugin(internal_plugins[i].path, &new_plugins);
+    LoadPlugin(internal_plugins[i].info.path, &new_plugins);
   }
 
   for (size_t i = 0; i < extra_plugin_paths.size(); ++i) {
diff --git a/webkit/plugins/npapi/plugin_list.h b/webkit/plugins/npapi/plugin_list.h
index 84026eb..bf9865e 100644
--- a/webkit/plugins/npapi/plugin_list.h
+++ b/webkit/plugins/npapi/plugin_list.h
@@ -43,25 +43,6 @@ struct PluginEntryPoints {
   NP_ShutdownFunc np_shutdown;
 };
 
-// This struct fully describes a plugin. For external plugins, it's read in from
-// the version info of the dll; For internal plugins, it's predefined and
-// includes addresses of entry functions. (Yes, it's Win32 NPAPI-centric, but
-// it'll do for holding descriptions of internal plugins cross-platform.)
-// TODO(evan): remove when NaCl is fixed.
-struct PluginVersionInfo {
-  FilePath path;
-  // Info about the plugin itself.
-  std::wstring product_name;
-  std::wstring file_description;
-  std::wstring file_version;
-  // Info about the data types that the plugin supports.
-  std::wstring mime_types;
-  std::wstring file_extensions;
-  std::wstring type_descriptions;
-  // Entry points for internal plugins.  Pointers are NULL for external plugins.
-  PluginEntryPoints entry_points;
-};
-
 // The PluginList is responsible for loading our NPAPI based plugins. It does
 // so in whatever manner is appropriate for the platform. On Windows, it loads
 // plugins from a known directory by looking for DLLs which start with "NP",
@@ -100,20 +81,22 @@ class PluginList {
   // Get the ordered list of directories from which to load plugins
   void GetPluginDirectories(std::vector<FilePath>* plugin_dirs);
 
-  // Register an internal plugin with the specified plugin information and
-  // function pointers.  An internal plugin must be registered before it can
+  // Register an internal plugin with the specified plugin information.
+  // An internal plugin must be registered before it can
   // be loaded using PluginList::LoadPlugin().
-  void RegisterInternalPlugin(const FilePath& path);
+  void RegisterInternalPlugin(const WebPluginInfo& info);
+
+  // This second version is for "plugins" that have been compiled
+  // directly into the binary -- callers must provide the metadata and
+  // the entry points.
+  // TODO(evan): we use file names here, but they're not really files, they're
+  // actually a string that uniquely identifies the plugin.
   void RegisterInternalPlugin(const FilePath& filename,
                               const std::string& name,
                               const std::string& description,
                               const std::string& mime_type,
                               const PluginEntryPoints& entry_points);
 
-  // Deprecated version of the above.
-  // TODO(evan): remove when NaCl is fixed.
-  void RegisterInternalPlugin(const PluginVersionInfo& info);
-
   // Removes a specified internal plugin from the list. The search will match
   // on the path from the version info previously registered.
   //
@@ -129,9 +112,15 @@ class PluginList {
                       WebPluginInfo* info,
                       const PluginEntryPoints** entry_points);
 
-  // Populate a WebPluginInfo from a PluginVersionInfo.
-  static bool CreateWebPluginInfo(const PluginVersionInfo& pvi,
-                                  WebPluginInfo* info);
+  // In Windows and Pepper plugins, the mime types are passed as a specially
+  // formatted list of strings.  This function parses those strings into
+  // a WebPluginMimeType vector.
+  // TODO(evan): make Pepper pass around formatted data and move this code
+  // into plugin_list_win.
+  static bool ParseMimeTypes(const std::string& mime_types,
+                             const std::string& file_extensions,
+                             const string16& mime_type_descriptions,
+                             std::vector<WebPluginMimeType>* parsed_mime_types);
 
   // Shutdown all plugins.  Should be called at process teardown.
   void Shutdown();
@@ -318,8 +307,12 @@ class PluginList {
   // Extra plugin directories that we want to search when loading.
   std::vector<FilePath> extra_plugin_dirs_;
 
+  struct InternalPlugin {
+    WebPluginInfo info;
+    PluginEntryPoints entry_points;
+  };
   // Holds information about internal plugins.
-  std::vector<PluginVersionInfo> internal_plugins_;
+  std::vector<InternalPlugin> internal_plugins_;
 
   // Path names of plugins to disable (the default is to enable them all).
   std::set<FilePath> disabled_plugins_;