Policy: Re-enabled plugin still disabled

chrome/browser/plugin_updater.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 "chrome/browser/plugin_updater.h"
 
 #include <set>
 #include <string>
 
 #include "base/command_line.h"
@@ skipping 48 matching lines @@
 void PluginUpdater::EnablePluginGroup(bool enable, const string16& group_name) {
   if (PluginGroup::IsPluginNameDisabledByPolicy(group_name))
     enable = false;
   NPAPI::PluginList::Singleton()->EnableGroup(enable, group_name);
   NotifyPluginStatusChanged();
 }
 
 void PluginUpdater::EnablePluginFile(bool enable,
                                      const FilePath::StringType& path) {
   FilePath file_path(path);
-  if (enable && !PluginGroup::IsPluginPathDisabledByPolicy(file_path))
+  bool disabled_by_policy =
+      PluginGroup::IsPluginPathDisabledByPolicy(file_path);
+  if (enable && !disabled_by_policy)
     NPAPI::PluginList::Singleton()->EnablePlugin(file_path);
   else
-    NPAPI::PluginList::Singleton()->DisablePlugin(file_path);
+    NPAPI::PluginList::Singleton()->DisablePlugin(file_path,
+                                                  disabled_by_policy);
 
   NotifyPluginStatusChanged();
 }
 
 void PluginUpdater::Observe(NotificationType type,
                             const NotificationSource& source,
                             const NotificationDetails& details) {
   DCHECK_EQ(NotificationType::PREF_CHANGED, type.value);
   const std::string* pref_name = Details<std::string>(details).ptr();
   if (!pref_name) {
@@ skipping 100 matching lines @@
           if (!enabled) {
             if (force_enable_internal_pdf) {
               enabled = true;
               plugin->SetBoolean("enabled", true);
             } else if (force_internal_pdf_for_this_run) {
               enabled = true;
             }
           }
         }
         if (!enabled)
-          NPAPI::PluginList::Singleton()->DisablePlugin(plugin_path);
+          NPAPI::PluginList::Singleton()->DisablePlugin(plugin_path, false);
       } else if (!enabled && plugin->GetString("name", &group_name)) {
         // Don't disable this group if it's for the pdf plugin and we just
         // forced it on.
         if (force_enable_internal_pdf && pdf_group_name == group_name)
           continue;
 
         // Otherwise this is a list of groups.
         EnablePluginGroup(false, group_name);
       }
     }
   }
 
   // Build the set of policy-disabled plugin patterns once and cache it.
   // Don't do this in the constructor, there's no profile available there.
   const ListValue* plugin_blacklist =
       profile->GetPrefs()->GetList(prefs::kPluginsPluginsBlacklist);
   DisablePluginsFromPolicy(plugin_blacklist);
 
   if ((!enable_internal_pdf_ && !found_internal_pdf) &&
       !force_internal_pdf_for_this_run) {
     // The internal PDF plugin is disabled by default, and the user hasn't
     // overridden the default.
-    NPAPI::PluginList::Singleton()->DisablePlugin(pdf_path);
+    NPAPI::PluginList::Singleton()->DisablePlugin(pdf_path, false);
     EnablePluginGroup(false, pdf_group_name);
   }
 
   if (force_enable_internal_pdf) {
     // See http://crbug.com/50105 for background.
     EnablePluginGroup(false, ASCIIToUTF16(PluginGroup::kAdobeReaderGroupName));
 
     // We want to save this, but doing so requires loading the list of plugins,
     // so do it after a minute as to not impact startup performance.  Note that
     // plugins are loaded after 30s by the metrics service.
     UpdatePreferences(profile, kPluginUpdateDelayMs);
   }
 }
 
 void PluginUpdater::UpdatePreferences(Profile* profile, int delay_ms) {
   BrowserThread::PostDelayedTask(
     BrowserThread::FILE,
     FROM_HERE,
     NewRunnableFunction(
         &PluginUpdater::GetPreferencesDataOnFileThread, profile), delay_ms);
 }
 
 void PluginUpdater::GetPreferencesDataOnFileThread(void* profile) {
   std::vector<WebPluginInfo> plugins;
   NPAPI::PluginList::Singleton()->GetPlugins(false, &plugins);
+  NPAPI::PluginList::DisabledPluginsList disabled_plugins;
+  NPAPI::PluginList::Singleton()->GetDisabledPlugins(&disabled_plugins);
 
   std::vector<PluginGroup> groups;
   NPAPI::PluginList::Singleton()->GetPluginGroups(false, &groups);
+  NPAPI::PluginList::DisabledGroupsList disabled_groups;
+  NPAPI::PluginList::Singleton()->GetDisabledGroups(&disabled_groups);
 
   BrowserThread::PostTask(
     BrowserThread::UI,
     FROM_HERE,
     NewRunnableFunction(
         &PluginUpdater::OnUpdatePreferences,
-        static_cast<Profile*>(profile), plugins, groups));
+        static_cast<Profile*>(profile),
+        plugins, groups,
+        disabled_plugins, disabled_groups));
 }
 
 void PluginUpdater::OnUpdatePreferences(
     Profile* profile,
     const std::vector<WebPluginInfo>& plugins,
-    const std::vector<PluginGroup>& groups) {
+    const std::vector<PluginGroup>& groups,
+    const NPAPI::PluginList::DisabledPluginsList& disabled_plugins,
+    const NPAPI::PluginList::DisabledGroupsList& disabled_groups) {
   ListValue* plugins_list = profile->GetPrefs()->GetMutableList(
       prefs::kPluginsPluginsList);
   plugins_list->Clear();
 
   FilePath internal_dir;
   if (PathService::Get(chrome::DIR_INTERNAL_PLUGINS, &internal_dir))
     profile->GetPrefs()->SetFilePath(prefs::kPluginsLastInternalDirectory,
                                      internal_dir);
 
   // Add the plugin files.
   for (std::vector<WebPluginInfo>::const_iterator it = plugins.begin();
        it != plugins.end();
        ++it) {
-    plugins_list->Append(CreatePluginFileSummary(*it));
+    DictionaryValue* summary = CreatePluginFileSummary(*it);
+    NPAPI::PluginList::DisabledPluginsList::const_iterator entry =
+        disabled_plugins.find(it->path);
+    // If the plugin is disabled only by policy don't store this state in the
+    // user pref store.
+    if (entry != disabled_plugins.end() &&
+        entry->second == NPAPI::PluginList::POLICY)

[danno, 2010/12/14 09:43:39]

Add methods to check this so the code looks more like
enter->second->IsDisabledByPolicy. Then maybe you can make the enum private.

[pastarmovj, 2010/12/15 14:44:51]

This doesn't seem to me to bring any more strucure to the code. Please correct
me if I am wrong. (Maybe in a short offline chat).

+      summary->SetBoolean("enabled", true);
+    plugins_list->Append(summary);
   }
 
   // Add the groups as well.
-  for (size_t i = 0; i < groups.size(); ++i) {
+  for (std::vector<PluginGroup>::const_iterator it = groups.begin();
+       it != groups.end();
+       ++it) {
     // Don't save preferences for vulnerable pugins.
     if (!CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableOutdatedPlugins) ||
-        !groups[i].IsVulnerable()) {
-      plugins_list->Append(groups[i].GetSummary());
+        !it->IsVulnerable()) {

[danno, 2010/12/14 09:43:39]

Might make code more readable to switch order of terms in ||

[pastarmovj, 2010/12/15 14:44:51]

Done.

+      DictionaryValue* summary = it->GetSummary();
+      NPAPI::PluginList::DisabledGroupsList::const_iterator entry =
+          disabled_groups.find(it->GetGroupName());
+      // If the plugin is disabled only by policy don't store this state in the
+      // user pref store.
+      if (entry != disabled_groups.end() &&
+          entry->second == NPAPI::PluginList::POLICY)
+        summary->SetBoolean("enabled", true);
+      plugins_list->Append(summary);
     }
   }
 }
 
 void PluginUpdater::NotifyPluginStatusChanged() {
   if (notify_pending_)
     return;
   notify_pending_ = true;
   MessageLoop::current()->PostTask(
       FROM_HERE,
       NewRunnableFunction(&PluginUpdater::OnNotifyPluginStatusChanged));
 }
 
 void PluginUpdater::OnNotifyPluginStatusChanged() {
   GetInstance()->notify_pending_ = false;
   NotificationService::current()->Notify(
       NotificationType::PLUGIN_ENABLE_STATUS_CHANGED,
       Source<PluginUpdater>(GetInstance()),
       NotificationService::NoDetails());
 }
 
 /*static*/
 PluginUpdater* PluginUpdater::GetInstance() {
   return Singleton<PluginUpdater>::get();
 }