Policy: Re-enabled plugin still disabled

webkit/plugins/npapi/plugin_group.cc

 // Copyright (c) 2010 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 "webkit/plugins/npapi/plugin_group.h"
 
 #include "base/linked_ptr.h"
 #include "base/string_util.h"
 #include "base/sys_string_conversions.h"
 #include "base/utf_string_conversions.h"
@@ skipping 28 matching lines @@
       policy_disabled_plugin_patterns_->begin());
   while (pattern != policy_disabled_plugin_patterns_->end()) {
     if (MatchPattern(plugin_name, *pattern))
       return true;
     ++pattern;
   }
 
   return false;
 }
 
-/*static*/
-bool PluginGroup::IsPluginPathDisabledByPolicy(const FilePath& plugin_path) {
-  std::vector<WebPluginInfo> plugins;
-  PluginList::Singleton()->GetPlugins(false, &plugins);
-  for (std::vector<WebPluginInfo>::const_iterator it = plugins.begin();
-       it != plugins.end();
-       ++it) {
-    if (FilePath::CompareEqualIgnoreCase(it->path.value(),
-        plugin_path.value()) && IsPluginNameDisabledByPolicy(it->name)) {
-      return true;
-    }
-  }
-  return false;
-}
-
 VersionRange::VersionRange(VersionRangeDefinition definition)
     : low_str(definition.version_matcher_low),
       high_str(definition.version_matcher_high),
       min_str(definition.min_version) {
   if (!low_str.empty())
     low.reset(Version::GetVersionFromString(low_str));
   if (!high_str.empty())
     high.reset(Version::GetVersionFromString(high_str));
   if (!min_str.empty())
     min.reset(Version::GetVersionFromString(min_str));
@@ skipping 20 matching lines @@
 }
 
 PluginGroup::PluginGroup(const string16& group_name,
                          const string16& name_matcher,
                          const std::string& update_url,
                          const std::string& identifier)
     : identifier_(identifier),
       group_name_(group_name),
       name_matcher_(name_matcher),
       update_url_(update_url),
-      enabled_(false),
+      enabled_(true),
       version_(Version::GetVersionFromString("0")) {
 }
 
 void PluginGroup::InitFrom(const PluginGroup& other) {
   identifier_ = other.identifier_;
   group_name_ = other.group_name_;
   name_matcher_ = other.name_matcher_;
   description_ = other.description_;
   update_url_ = other.update_url_;
   enabled_ = other.enabled_;
   version_ranges_ = other.version_ranges_;
   version_.reset(other.version_->Clone());
   web_plugin_infos_ = other.web_plugin_infos_;
-  web_plugin_positions_ = other.web_plugin_positions_;
 }
 
 PluginGroup::PluginGroup(const PluginGroup& other) {
   InitFrom(other);
 }
 
 PluginGroup& PluginGroup::operator=(const PluginGroup& other) {
   InitFrom(other);
   return *this;
 }
@@ skipping 77 matching lines @@
   std::replace(version.begin(), version.end(), 'r', '.');
   std::replace(version.begin(), version.end(), ',', '.');
   std::replace(version.begin(), version.end(), '(', '.');
   std::replace(version.begin(), version.end(), '_', '.');
 
   return Version::GetVersionFromString(WideToASCII(version));
 }
 
 void PluginGroup::UpdateActivePlugin(const WebPluginInfo& plugin) {
   // A group is enabled if any of the files are enabled.
-  if (plugin.enabled) {
-    if (!enabled_) {
+  if (IsPluginEnabled(plugin)) {
+    if (!enabled_ || description_.empty()) {

[jam, 2011/01/19 20:22:09]

don't need to look for empty description anymore right?

[pastarmovj, 2011/01/19 23:39:17]

Actually I do now. This is not part of the enforce plugins to have descriptions
effort I had before. Here I actually had to introduce that as a counterpart to
the same check in the else clause because before we added groups as being
disabled and now I have them in both states depending on various criteria (user
disabled plugin,disabled group or policy). It just makes sure the group that has
no name gets one from it's first plugin no matter whether it is enabled or not
and what the group's state is. No real change in the behavior here only
capturing the possible group's enabled states.

[jam, 2011/01/20 00:24:25]

Got it.  Please add a comment as this is hard to understand when looking at the
code.  Can this logic be factored into two separate checks?  i.e. when to take
care of enabled_, and another of description?

       // If this is the first enabled plugin, use its description.
       enabled_ = true;
       UpdateDescriptionAndVersion(plugin);
     }
   } else {
     // If this is the first plugin and it's disabled,
     // use its description for now.
     if (description_.empty())
       UpdateDescriptionAndVersion(plugin);
   }
 }
 
 void PluginGroup::UpdateDescriptionAndVersion(const WebPluginInfo& plugin) {
   description_ = plugin.desc;
   if (Version* new_version = CreateVersionFromString(plugin.version))
     version_.reset(new_version);
   else
     version_.reset(Version::GetVersionFromString("0"));
 }
 
-void PluginGroup::AddPlugin(const WebPluginInfo& plugin, int position) {
+bool PluginGroup::AddPlugin(const WebPluginInfo& plugin, int priority) {

[jam, 2011/01/19 20:22:09]

priority isn't used anymore

[pastarmovj, 2011/01/19 23:39:17]

Done.

   // Check if this group already contains this plugin.
   for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
-    if (web_plugin_infos_[i].name == plugin.name &&
-        web_plugin_infos_[i].version == plugin.version &&
-        FilePath::CompareEqualIgnoreCase(web_plugin_infos_[i].path.value(),
+    if (FilePath::CompareEqualIgnoreCase(web_plugin_infos_[i].path.value(),
                                          plugin.path.value())) {
-      return;
+      return false;
     }
   }
   web_plugin_infos_.push_back(plugin);
-  // The position of this plugin relative to the global list of plugins.
-  web_plugin_positions_.push_back(position);
-  UpdateActivePlugin(plugin);
+  // If the group is disabled disable new plugins in it too. This should cover
+  // the case where a plugin has been upgraded that has been disabled and should
+  // stay disabled after the upgrade.
+  if (!enabled_)
+    DisablePlugin(web_plugin_infos_.back().path);
+  UpdateActivePlugin(web_plugin_infos_.back());
+  RefreshEnabledState();
+  return true;
 }
 
-bool PluginGroup::IsEmpty() const {
-  return web_plugin_infos_.empty();
+bool PluginGroup::RemovePlugin(const FilePath& filename) {
+  for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
+    if (web_plugin_infos_[i].path == filename) {
+      web_plugin_infos_.erase(web_plugin_infos_.begin() + i);
+      return true;
+    }
+  }
+  return false;
+}
+
+bool PluginGroup::EnablePlugin(const FilePath& filename) {
+  for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
+    if (web_plugin_infos_[i].path == filename) {
+      bool did_enable = Enable(&web_plugin_infos_[i],
+                               WebPluginInfo::USER_ENABLED);
+      RefreshEnabledState();
+      return did_enable;
+    }
+  }
+  return false;
+}
+
+bool PluginGroup::DisablePlugin(const FilePath& filename) {
+  for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
+    if (web_plugin_infos_[i].path == filename) {
+      // We are only called for user intervention however we should respect a
+      // policy that might as well be active on this plugin.
+      bool did_disable = Disable(
+          &web_plugin_infos_[i],
+          IsPluginNameDisabledByPolicy(web_plugin_infos_[i].name) ?
+              WebPluginInfo::USER_DISABLED_POLICY_DISABLED :
+              WebPluginInfo::USER_DISABLED);
+      RefreshEnabledState();
+      return did_disable;
+    }
+  }
+  return false;
 }
 
 string16 PluginGroup::GetGroupName() const {
   if (!group_name_.empty())
     return group_name_;
   DCHECK_EQ(1u, web_plugin_infos_.size());
   FilePath::StringType path =
-      web_plugin_infos_[0].path.BaseName().RemoveExtension().value();
+      web_plugin_infos_.front().path.BaseName().RemoveExtension().value();

[jam, 2011/01/19 20:22:09]

nit: any reason to change this?  we avoid changing code in changes to match
personal style, as that leads to a lot of hysteresis if everyone does it.

[pastarmovj, 2011/01/19 23:39:17]

Relict from the times this wasn't a vector. Reverted.

 #if defined(OS_POSIX)
   return UTF8ToUTF16(path);
 #elif defined(OS_WIN)
   return WideToUTF16(path);
 #endif
 }
 
+const std::vector<WebPluginInfo>& PluginGroup::GetPlugins() const {
+  return web_plugin_infos_;
+}
+
+bool PluginGroup::ContainsPlugin(const FilePath& path) const {
+  for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
+    if (web_plugin_infos_[i].path == path)
+      return true;
+  }
+  return false;
+}
+
+
 DictionaryValue* PluginGroup::GetSummary() const {
   DictionaryValue* result = new DictionaryValue();
   result->SetString("name", GetGroupName());
   result->SetBoolean("enabled", enabled_);
   return result;
 }
 
 DictionaryValue* PluginGroup::GetDataForUI() const {
   string16 name = GetGroupName();
   DictionaryValue* result = new DictionaryValue();
   result->SetString("name", name);
   result->SetString("description", description_);
   result->SetString("version", version_->GetString());
   result->SetString("update_url", update_url_);
   result->SetBoolean("critical", IsVulnerable());
 
   bool group_disabled_by_policy = IsPluginNameDisabledByPolicy(name);
   ListValue* plugin_files = new ListValue();
   bool all_plugins_disabled_by_policy = true;
   for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
-    const WebPluginInfo& web_plugin = web_plugin_infos_[i];
-    int priority = web_plugin_positions_[i];
     DictionaryValue* plugin_file = new DictionaryValue();
-    plugin_file->SetString("name", web_plugin.name);
-    plugin_file->SetString("description", web_plugin.desc);
-    plugin_file->SetString("path", web_plugin.path.value());
-    plugin_file->SetString("version", web_plugin.version);
+    plugin_file->SetString("name", web_plugin_infos_[i].name);
+    plugin_file->SetString("description", web_plugin_infos_[i].desc);
+    plugin_file->SetString("path", web_plugin_infos_[i].path.value());
+    plugin_file->SetString("version", web_plugin_infos_[i].version);
     bool plugin_disabled_by_policy = group_disabled_by_policy ||
-        IsPluginNameDisabledByPolicy(web_plugin.name);
+        ((web_plugin_infos_[i].enabled & WebPluginInfo::POLICY_DISABLED) != 0);
     if (plugin_disabled_by_policy) {
       plugin_file->SetString("enabledMode", "disabledByPolicy");
     } else {
       all_plugins_disabled_by_policy = false;
-      plugin_file->SetString("enabledMode",
-                             web_plugin.enabled ? "enabled" : "disabledByUser");
+      plugin_file->SetString(
+          "enabledMode", IsPluginEnabled(web_plugin_infos_[i]) ?
+              "enabled" : "disabledByUser");
     }
-    plugin_file->SetInteger("priority", priority);
 
     ListValue* mime_types = new ListValue();
-    for (std::vector<WebPluginMimeType>::const_iterator type_it =
-         web_plugin.mime_types.begin();
-         type_it != web_plugin.mime_types.end();
-         ++type_it) {
+    const std::vector<WebPluginMimeType>& plugin_mime_types =
+        web_plugin_infos_[i].mime_types;
+    for (size_t j = 0; j < plugin_mime_types.size(); ++j) {
       DictionaryValue* mime_type = new DictionaryValue();
-      mime_type->SetString("mimeType", type_it->mime_type);
-      mime_type->SetString("description", type_it->description);
+      mime_type->SetString("mimeType", plugin_mime_types[j].mime_type);
+      mime_type->SetString("description", plugin_mime_types[j].description);
 
       ListValue* file_extensions = new ListValue();
-      for (std::vector<std::string>::const_iterator ext_it =
-           type_it->file_extensions.begin();
-           ext_it != type_it->file_extensions.end();
-           ++ext_it) {
-        file_extensions->Append(new StringValue(*ext_it));
-      }
+      const std::vector<std::string>& mime_file_extensions =
+          plugin_mime_types[j].file_extensions;
+      for (size_t k = 0; k < mime_file_extensions.size(); ++k)
+        file_extensions->Append(new StringValue(mime_file_extensions[k]));
       mime_type->Set("fileExtensions", file_extensions);
 
       mime_types->Append(mime_type);
     }
     plugin_file->Set("mimeTypes", mime_types);
 
     plugin_files->Append(plugin_file);
   }
 
   if (group_disabled_by_policy || all_plugins_disabled_by_policy) {
@@ skipping 30 matching lines @@
 
 // Returns true if the latest version of this plugin group is vulnerable.
 bool PluginGroup::IsVulnerable() const {
   for (size_t i = 0; i < version_ranges_.size(); ++i) {
     if (IsPluginOutdated(*version_, version_ranges_[i]))
       return true;
   }
   return false;
 }
 
+bool PluginGroup::IsEmpty() const {
+  return web_plugin_infos_.size() == 0;
+}
+
 void PluginGroup::DisableOutdatedPlugins() {
-  description_ = string16();
-  enabled_ = false;
+  bool first_enabled = true;
 
-  for (std::vector<WebPluginInfo>::iterator it =
-           web_plugin_infos_.begin();
-       it != web_plugin_infos_.end(); ++it) {
-    scoped_ptr<Version> version(CreateVersionFromString(it->version));
+  for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
+    scoped_ptr<Version> version(
+        CreateVersionFromString(web_plugin_infos_[i].version));
     if (version.get()) {
-      for (size_t i = 0; i < version_ranges_.size(); ++i) {
-        if (IsPluginOutdated(*version, version_ranges_[i])) {
-          it->enabled = false;
-          PluginList::Singleton()->DisablePlugin(it->path);
+      bool plugin_is_outdated = false;
+      for (size_t j = 0; j < version_ranges_.size(); ++j) {
+        if (IsPluginOutdated(*version, version_ranges_[j])) {
+          Disable(&web_plugin_infos_[i], WebPluginInfo::USER_DISABLED);
+          plugin_is_outdated = true;
+          break;
         }
       }
+      if (!plugin_is_outdated && first_enabled) {
+        first_enabled = false;
+        UpdateDescriptionAndVersion(web_plugin_infos_[i]);
+      }
     }
-    UpdateActivePlugin(*it);
   }
 }
 
-void PluginGroup::Enable(bool enable) {
+bool PluginGroup::EnableGroup(bool enable) {
   bool enabled_plugin_exists = false;
-  for (std::vector<WebPluginInfo>::iterator it =
-       web_plugin_infos_.begin();
-       it != web_plugin_infos_.end(); ++it) {
-    if (enable && !IsPluginNameDisabledByPolicy(it->name)) {
-      PluginList::Singleton()->EnablePlugin(it->path);
-      it->enabled = true;
+  bool group_disabled_by_policy = IsPluginNameDisabledByPolicy(group_name_);
+  // We can't enable groups disabled by policy
+  if (group_disabled_by_policy && enable)
+    return false;
+
+  for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
+    bool policy_disabled =
+        IsPluginNameDisabledByPolicy(web_plugin_infos_[i].name);
+    if (enable && !policy_disabled) {
+      Enable(&web_plugin_infos_[i], WebPluginInfo::USER_ENABLED);
       enabled_plugin_exists = true;
     } else {
-      it->enabled = false;
-      PluginList::Singleton()->DisablePlugin(it->path);
+      Disable(&web_plugin_infos_[i],
+              policy_disabled || group_disabled_by_policy ?
+                  WebPluginInfo::POLICY_DISABLED :
+                  WebPluginInfo::USER_DISABLED);
+    }
+  }
+  enabled_ = enabled_plugin_exists;
+  return enabled_ == enable;
+}
+
+void PluginGroup::EnforceGroupPolicy() {
+  bool enabled_plugin_exists = false;
+  bool group_disabled_by_policy = IsPluginNameDisabledByPolicy(group_name_);
+
+  for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
+    bool policy_disabled =
+        IsPluginNameDisabledByPolicy(web_plugin_infos_[i].name) |
+        group_disabled_by_policy;
+
+    // TODO(pastarmovj): Add the code for enforcing enabled by policy...
+    if (policy_disabled) {
+      Disable(&web_plugin_infos_[i], WebPluginInfo::POLICY_DISABLED);
+    // ...here would a else if (policy_enabled) { ... } be then.
+    } else {
+      Enable(&web_plugin_infos_[i], WebPluginInfo::POLICY_UNMANAGED);
+      if (IsPluginEnabled(web_plugin_infos_[i]))
+        enabled_plugin_exists = true;
     }
   }
   enabled_ = enabled_plugin_exists;
 }
 
+void PluginGroup::RefreshEnabledState() {
+  bool enabled_plugin_exists = false;
+  for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
+    if (IsPluginEnabled(web_plugin_infos_[i])) {
+      enabled_plugin_exists = true;
+      break;
+    }
+  }
+  enabled_ = enabled_plugin_exists;
+}
+
+bool PluginGroup::Enable(WebPluginInfo* plugin,
+                         int new_reason) {
+  DCHECK(new_reason == WebPluginInfo::USER_ENABLED ||
+         new_reason == WebPluginInfo::POLICY_UNMANAGED ||
+         new_reason == WebPluginInfo::POLICY_ENABLED);
+  // If we are only stripping the policy then mask the policy bits.
+  if (new_reason == WebPluginInfo::POLICY_UNMANAGED) {
+    plugin->enabled &= WebPluginInfo::USER_MASK;
+    return true;
+  }
+  // If already enabled just upgrade the reason.
+  if (IsPluginEnabled(*plugin)) {
+    plugin->enabled |= new_reason;
+    return true;
+  } else {
+    // Only changeable if not managed.
+    if (plugin->enabled & WebPluginInfo::MANAGED_MASK)
+      return false;
+    plugin->enabled = new_reason;
+  }
+  return true;
+}
+
+bool PluginGroup::Disable(WebPluginInfo* plugin,
+                          int new_reason) {
+  DCHECK(new_reason == WebPluginInfo::USER_DISABLED ||
+         new_reason == WebPluginInfo::POLICY_DISABLED);
+  // If already disabled just upgrade the reason.
+  if (!IsPluginEnabled(*plugin)) {
+    plugin->enabled |= new_reason;
+    return true;
+  } else {
+    // Only changeable if not managed.
+    if (plugin->enabled & WebPluginInfo::MANAGED_MASK)
+      return false;
+    plugin->enabled = new_reason;
+  }
+  return true;
+}
+
 }  // namespace npapi
 }  // namespace webkit