Fix plug-in selection logic when the default plug-in is disabled.

BUG=none
TEST=The default plug-in is not used when it's disabled.


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

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

4 files changed, 275 insertions, 71 deletions

diff --git a/chrome/browser/renderer_host/plugin_info_message_filter.cc b/chrome/browser/renderer_host/plugin_info_message_filter.cc
index 2052a77..1b85efd5 100644
--- a/chrome/browser/renderer_host/plugin_info_message_filter.cc
+++ b/chrome/browser/renderer_host/plugin_info_message_filter.cc
@@ -60,13 +60,11 @@ void PluginInfobarExperiment(bool* allow_outdated,
 
 }  // namespace
 
-PluginInfoMessageFilter::PluginInfoMessageFilter(
-    int render_process_id,
-    Profile* profile)
-  : render_process_id_(render_process_id),
-    resource_context_(profile->GetResourceContext()),
-    host_content_settings_map_(profile->GetHostContentSettingsMap()),
-    weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+PluginInfoMessageFilter::Context::Context(int render_process_id,
+                                          Profile* profile)
+    : render_process_id_(render_process_id),
+      resource_context_(&profile->GetResourceContext()),
+      host_content_settings_map_(profile->GetHostContentSettingsMap()) {
   allow_outdated_plugins_.Init(prefs::kPluginsAllowOutdated,
                                profile->GetPrefs(), NULL);
   allow_outdated_plugins_.MoveToThread(content::BrowserThread::IO);
@@ -75,6 +73,22 @@ PluginInfoMessageFilter::PluginInfoMessageFilter(
   always_authorize_plugins_.MoveToThread(content::BrowserThread::IO);
 }
 
+PluginInfoMessageFilter::Context::Context()
+    : render_process_id_(0),
+      resource_context_(NULL),
+      host_content_settings_map_(NULL) {
+}
+
+PluginInfoMessageFilter::Context::~Context() {
+}
+
+PluginInfoMessageFilter::PluginInfoMessageFilter(
+    int render_process_id,
+    Profile* profile)
+    : context_(render_process_id, profile),
+      weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+}
+
 PluginInfoMessageFilter::~PluginInfoMessageFilter() {}
 
 bool PluginInfoMessageFilter::OnMessageReceived(const IPC::Message& message,
@@ -129,13 +143,13 @@ void PluginInfoMessageFilter::PluginsLoaded(
   ChromeViewHostMsg_GetPluginInfo_Status status;
   webkit::WebPluginInfo plugin;
   std::string actual_mime_type;
-  DecidePluginStatus(params, &status, &plugin, &actual_mime_type);
+  context_.DecidePluginStatus(params, &status, &plugin, &actual_mime_type);
   ChromeViewHostMsg_GetPluginInfo::WriteReplyParams(
       reply_msg, status, plugin, actual_mime_type);
   Send(reply_msg);
 }
 
-void PluginInfoMessageFilter::DecidePluginStatus(
+void PluginInfoMessageFilter::Context::DecidePluginStatus(
     const GetPluginInfo_Params& params,
     ChromeViewHostMsg_GetPluginInfo_Status* status,
     webkit::WebPluginInfo* plugin,
@@ -193,7 +207,7 @@ void PluginInfoMessageFilter::DecidePluginStatus(
     status->value = ChromeViewHostMsg_GetPluginInfo_Status::kBlocked;
 }
 
-bool PluginInfoMessageFilter::FindEnabledPlugin(
+bool PluginInfoMessageFilter::Context::FindEnabledPlugin(
     int render_view_id,
     const GURL& url,
     const GURL& top_origin_url,
@@ -206,49 +220,44 @@ bool PluginInfoMessageFilter::FindEnabledPlugin(
   std::vector<std::string> mime_types;
   PluginService::GetInstance()->GetPluginInfoArray(
       url, mime_type, allow_wildcard, &matching_plugins, &mime_types);
-  if (matching_plugins.empty()) {
-    status->value = ChromeViewHostMsg_GetPluginInfo_Status::kNotFound;
-    return true;
-  }
-
-  if (matching_plugins.size() > 1 &&
-      matching_plugins.back().path ==
-          FilePath(webkit::npapi::kDefaultPluginLibraryName)) {
-    // If there is at least one plug-in handling the required MIME type (apart
-    // from the default plug-in), we don't need the default plug-in.
-    matching_plugins.pop_back();
-  }
-
   content::PluginServiceFilter* filter =
       PluginService::GetInstance()->GetFilter();
-  bool allowed = false;
+  bool found = false;
   for (size_t i = 0; i < matching_plugins.size(); ++i) {
-    if (!filter || filter->ShouldUsePlugin(render_process_id_,
-                                           render_view_id,
-                                           &resource_context_,
-                                           url,
-                                           top_origin_url,
-                                           &matching_plugins[i])) {
-      *plugin = matching_plugins[i];
-      *actual_mime_type = mime_types[i];
-      allowed = true;
-      break;
-    } else if ((i == 0) &&
-               (matching_plugins[i].path !=
-                    FilePath(webkit::npapi::kDefaultPluginLibraryName))) {
+    bool enabled = !filter || filter->ShouldUsePlugin(render_process_id_,
+                                                      render_view_id,
+                                                      resource_context_,
+                                                      url,
+                                                      top_origin_url,
+                                                      &matching_plugins[i]);
+    bool is_default_plugin = matching_plugins[i].path ==
+                             FilePath(webkit::npapi::kDefaultPluginLibraryName);
+    if (enabled) {
+      if (!found || !is_default_plugin) {
+        // We have found an enabled plug-in. Return immediately.
+        *plugin = matching_plugins[i];
+        *actual_mime_type = mime_types[i];
+        return false;
+      }
+    } else if (!found && !is_default_plugin) {
+      // We have found a plug-in, but it's disabled. Keep looking for an
+      // enabled one.
       *plugin = matching_plugins[i];
       *actual_mime_type = mime_types[i];
+      found = true;
     }
   }
 
-  if (!allowed) {
+  // If we're here and have previously found a plug-in, it must have been
+  // disabled.
+  if (found)
     status->value = ChromeViewHostMsg_GetPluginInfo_Status::kDisabled;
-    return true;
-  }
-  return false;
+  else
+    status->value = ChromeViewHostMsg_GetPluginInfo_Status::kNotFound;
+  return true;
 }
 
-void PluginInfoMessageFilter::GetPluginContentSetting(
+void PluginInfoMessageFilter::Context::GetPluginContentSetting(
     const webkit::WebPluginInfo* plugin,
     const GURL& policy_url,
     const GURL& plugin_url,
diff --git a/chrome/browser/renderer_host/plugin_info_message_filter.h b/chrome/browser/renderer_host/plugin_info_message_filter.h
index bdafda7..3500bce 100644
--- a/chrome/browser/renderer_host/plugin_info_message_filter.h
+++ b/chrome/browser/renderer_host/plugin_info_message_filter.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -31,6 +31,43 @@ struct WebPluginInfo;
 // This class filters out incoming IPC messages requesting plug-in information.
 class PluginInfoMessageFilter : public content::BrowserMessageFilter {
  public:
+  struct GetPluginInfo_Params;
+
+  // Contains all the information needed by the PluginInfoMessageFilter.
+  class Context {
+   public:
+    Context(int render_process_id, Profile* profile);
+    // Dummy constructor for tests.
+    Context();
+    ~Context();
+
+    void DecidePluginStatus(const GetPluginInfo_Params& params,
+                            ChromeViewHostMsg_GetPluginInfo_Status* status,
+                            webkit::WebPluginInfo* plugin,
+                            std::string* actual_mime_type) const;
+    bool FindEnabledPlugin(int render_view_id,
+                           const GURL& url,
+                           const GURL& top_origin_url,
+                           const std::string& mime_type,
+                           ChromeViewHostMsg_GetPluginInfo_Status* status,
+                           webkit::WebPluginInfo* plugin,
+                           std::string* actual_mime_type) const;
+    void GetPluginContentSetting(const webkit::WebPluginInfo* plugin,
+                                 const GURL& policy_url,
+                                 const GURL& plugin_url,
+                                 const std::string& resource,
+                                 ContentSetting* setting,
+                                 bool* is_default) const;
+
+   private:
+    int render_process_id_;
+    const content::ResourceContext* resource_context_;
+    const HostContentSettingsMap* host_content_settings_map_;
+
+    BooleanPrefMember allow_outdated_plugins_;
+    BooleanPrefMember always_authorize_plugins_;
+  };
+
   PluginInfoMessageFilter(int render_process_id, Profile* profile);
   virtual ~PluginInfoMessageFilter();
 
@@ -40,45 +77,19 @@ class PluginInfoMessageFilter : public content::BrowserMessageFilter {
   virtual void OnDestruct() const OVERRIDE;
 
  private:
-  struct GetPluginInfo_Params;
-
   void OnGetPluginInfo(int render_view_id,
                        const GURL& url,
                        const GURL& top_origin_url,
                        const std::string& mime_type,
                        IPC::Message* reply_msg);
 
-  // Helper functions for |OnGetPluginInfo()|.
-
   // |params| wraps the parameters passed to |OnGetPluginInfo|, because
   // |base::Bind| doesn't support the required arity <http://crbug.com/98542>.
   void PluginsLoaded(const GetPluginInfo_Params& params,
                      IPC::Message* reply_msg,
                      const std::vector<webkit::WebPluginInfo>& plugins);
-  void DecidePluginStatus(const GetPluginInfo_Params& params,
-                          ChromeViewHostMsg_GetPluginInfo_Status* status,
-                          webkit::WebPluginInfo* plugin,
-                          std::string* actual_mime_type) const;
-  bool FindEnabledPlugin(int render_view_id,
-                         const GURL& url,
-                         const GURL& top_origin_url,
-                         const std::string& mime_type,
-                         ChromeViewHostMsg_GetPluginInfo_Status* status,
-                         webkit::WebPluginInfo* plugin,
-                         std::string* actual_mime_type) const;
-  void GetPluginContentSetting(const webkit::WebPluginInfo* plugin,
-                               const GURL& policy_url,
-                               const GURL& plugin_url,
-                               const std::string& resource,
-                               ContentSetting* setting,
-                               bool* non_default) const;
-
-  int render_process_id_;
-  const content::ResourceContext& resource_context_;
-  const HostContentSettingsMap* host_content_settings_map_;
-
-  BooleanPrefMember allow_outdated_plugins_;
-  BooleanPrefMember always_authorize_plugins_;
+
+  Context context_;
 
   base::WeakPtrFactory<PluginInfoMessageFilter> weak_ptr_factory_;
 
diff --git a/chrome/browser/renderer_host/plugin_info_message_filter_unittest.cc b/chrome/browser/renderer_host/plugin_info_message_filter_unittest.cc
new file mode 100644
index 0000000..5e4a1cf
--- /dev/null
+++ b/chrome/browser/renderer_host/plugin_info_message_filter_unittest.cc
@@ -0,0 +1,183 @@
+// 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.
+
+#include "chrome/browser/renderer_host/plugin_info_message_filter.h"
+
+#include "base/at_exit.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/message_loop.h"
+#include "base/utf_string_conversions.h"
+#include "chrome/common/render_messages.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/browser/plugin_service_filter.h"
+#include "content/public/browser/plugin_service.h"
+#include "content/test/test_browser_thread.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/plugins/npapi/mock_plugin_list.h"
+
+using content::PluginService;
+
+namespace {
+
+class FakePluginServiceFilter : public content::PluginServiceFilter {
+ public:
+  FakePluginServiceFilter() {}
+  virtual ~FakePluginServiceFilter() {}
+
+  virtual bool ShouldUsePlugin(int render_process_id,
+                               int render_view_id,
+                               const void* context,
+                               const GURL& url,
+                               const GURL& policy_url,
+                               webkit::WebPluginInfo* plugin) OVERRIDE;
+
+  void set_plugin_enabled(const FilePath& plugin_path, bool enabled) {
+    plugin_state_[plugin_path] = enabled;
+  }
+
+ private:
+  std::map<FilePath, bool> plugin_state_;
+};
+
+bool FakePluginServiceFilter::ShouldUsePlugin(int render_process_id,
+                                              int render_view_id,
+                                              const void* context,
+                                              const GURL& url,
+                                              const GURL& policy_url,
+                                              webkit::WebPluginInfo* plugin) {
+  std::map<FilePath, bool>::iterator it = plugin_state_.find(plugin->path);
+  if (it == plugin_state_.end()) {
+    ADD_FAILURE() << "No plug-in state for '" << plugin->path.value() << "'";
+    return false;
+  }
+  return it->second;
+}
+
+}  // namespace
+
+class PluginInfoMessageFilterTest : public ::testing::Test {
+ public:
+  PluginInfoMessageFilterTest() :
+      default_plugin_path_(webkit::npapi::kDefaultPluginLibraryName),
+      foo_plugin_path_(FILE_PATH_LITERAL("/path/to/foo")),
+      bar_plugin_path_(FILE_PATH_LITERAL("/path/to/bar")),
+      file_thread_(content::BrowserThread::FILE, &message_loop_),
+      plugin_list_(NULL, 0) {
+  }
+
+  virtual void SetUp() OVERRIDE {
+    webkit::WebPluginInfo foo_plugin(ASCIIToUTF16("Foo Plug-in"),
+                                     foo_plugin_path_,
+                                     ASCIIToUTF16("1"),
+                                     ASCIIToUTF16("The Foo plug-in."));
+    webkit::WebPluginMimeType mimeType;
+    mimeType.mime_type = "foo/bar";
+    foo_plugin.mime_types.push_back(mimeType);
+    plugin_list_.AddPluginToLoad(foo_plugin);
+
+    webkit::WebPluginInfo bar_plugin(ASCIIToUTF16("Bar Plug-in"),
+                                     bar_plugin_path_,
+                                     ASCIIToUTF16("1"),
+                                     ASCIIToUTF16("The Bar plug-in."));
+    mimeType.mime_type = "foo/bar";
+    bar_plugin.mime_types.push_back(mimeType);
+    plugin_list_.AddPluginToLoad(bar_plugin);
+
+    webkit::WebPluginInfo default_plugin(
+        ASCIIToUTF16("Default Plug-in"),
+        default_plugin_path_,
+        ASCIIToUTF16("1"),
+        ASCIIToUTF16("Provides functionality for installing third-party "
+                     "plug-ins"));
+    mimeType.mime_type = "*";
+    default_plugin.mime_types.push_back(mimeType);
+    plugin_list_.AddPluginToLoad(default_plugin);
+
+    PluginService::GetInstance()->SetPluginListForTesting(&plugin_list_);
+    PluginService::GetInstance()->SetFilter(&filter_);
+
+    PluginService::GetInstance()->GetPlugins(
+        base::Bind(&PluginInfoMessageFilterTest::PluginsLoaded,
+                   base::Unretained(this)));
+    ui_test_utils::RunMessageLoop();
+  }
+
+ protected:
+  FilePath default_plugin_path_;
+  FilePath foo_plugin_path_;
+  FilePath bar_plugin_path_;
+  FakePluginServiceFilter filter_;
+  PluginInfoMessageFilter::Context context_;
+
+ private:
+  void PluginsLoaded(const std::vector<webkit::WebPluginInfo>& plugins) {
+    MessageLoop::current()->Quit();
+  }
+
+  MessageLoop message_loop_;
+  // PluginService::GetPlugins on Windows jumps to the FILE thread even with
+  // a MockPluginList.
+  content::TestBrowserThread file_thread_;
+  base::ShadowingAtExitManager at_exit_manager_;  // Destroys the PluginService.
+  webkit::npapi::MockPluginList plugin_list_;
+};
+
+TEST_F(PluginInfoMessageFilterTest, FindEnabledPlugin) {
+  filter_.set_plugin_enabled(foo_plugin_path_, true);
+  filter_.set_plugin_enabled(bar_plugin_path_, true);
+  filter_.set_plugin_enabled(default_plugin_path_, true);
+  {
+    ChromeViewHostMsg_GetPluginInfo_Status status;
+    webkit::WebPluginInfo plugin;
+    std::string actual_mime_type;
+    EXPECT_FALSE(context_.FindEnabledPlugin(
+        0, GURL(), GURL(), "foo/bar", &status, &plugin, &actual_mime_type));
+    EXPECT_EQ(ChromeViewHostMsg_GetPluginInfo_Status::kAllowed, status.value);
+    EXPECT_EQ(foo_plugin_path_.value(), plugin.path.value());
+  }
+  {
+    ChromeViewHostMsg_GetPluginInfo_Status status;
+    webkit::WebPluginInfo plugin;
+    std::string actual_mime_type;
+    EXPECT_FALSE(context_.FindEnabledPlugin(
+        0, GURL(), GURL(), "baz/blurp", &status, &plugin, &actual_mime_type));
+    EXPECT_EQ(ChromeViewHostMsg_GetPluginInfo_Status::kAllowed, status.value);
+    EXPECT_EQ(default_plugin_path_.value(), plugin.path.value());
+  }
+
+  filter_.set_plugin_enabled(foo_plugin_path_, false);
+  {
+    ChromeViewHostMsg_GetPluginInfo_Status status;
+    webkit::WebPluginInfo plugin;
+    std::string actual_mime_type;
+    EXPECT_FALSE(context_.FindEnabledPlugin(
+        0, GURL(), GURL(), "foo/bar", &status, &plugin, &actual_mime_type));
+    EXPECT_EQ(ChromeViewHostMsg_GetPluginInfo_Status::kAllowed, status.value);
+    EXPECT_EQ(bar_plugin_path_.value(), plugin.path.value());
+  }
+
+  filter_.set_plugin_enabled(bar_plugin_path_, false);
+  {
+    ChromeViewHostMsg_GetPluginInfo_Status status;
+    webkit::WebPluginInfo plugin;
+    std::string actual_mime_type;
+    EXPECT_TRUE(context_.FindEnabledPlugin(
+        0, GURL(), GURL(), "foo/bar", &status, &plugin, &actual_mime_type));
+    EXPECT_EQ(ChromeViewHostMsg_GetPluginInfo_Status::kDisabled, status.value);
+    EXPECT_EQ(foo_plugin_path_.value(), plugin.path.value());
+  }
+
+  filter_.set_plugin_enabled(default_plugin_path_, false);
+  {
+    ChromeViewHostMsg_GetPluginInfo_Status status;
+    webkit::WebPluginInfo plugin;
+    std::string actual_mime_type;
+    EXPECT_TRUE(context_.FindEnabledPlugin(
+        0, GURL(), GURL(), "baz/blurp", &status, &plugin, &actual_mime_type));
+    EXPECT_EQ(ChromeViewHostMsg_GetPluginInfo_Status::kNotFound, status.value);
+    EXPECT_EQ(FILE_PATH_LITERAL(""), plugin.path.value());
+  }
+}
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 53fed2f..f02420b 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1635,6 +1635,7 @@
         'browser/profiles/profile_info_util_unittest.cc',
         'browser/profiles/profile_manager_unittest.cc',
         'browser/renderer_host/web_cache_manager_unittest.cc',
+        'browser/renderer_host/plugin_info_message_filter_unittest.cc',
         'browser/resources/print_preview/print_preview_utils.js',
         'browser/resources/print_preview/print_preview_utils_unittest.gtestjs',
         'browser/resources_util_unittest.cc',