Destroy configurations created by VPN extension on disable

extensions/browser/api/vpn_provider/vpn_service.cc

 // Copyright 2014 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 "extensions/browser/api/vpn_provider/vpn_service.h"
 
 #include <stdint.h>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 413 matching lines @@
     const std::string& configuration_name) {
   const std::string key = GetKey(extension_id, configuration_name);
   return ContainsKey(key_to_configuration_map_, key);
 }
 
 bool VpnService::VerifyConfigIsConnectedForTesting(
     const std::string& extension_id) {
   return DoesActiveConfigurationExistAndIsAccessAuthorized(extension_id);
 }
 
-void VpnService::OnExtensionUninstalled(
-    content::BrowserContext* browser_context,
-    const extensions::Extension* extension,
-    extensions::UninstallReason reason) {
-  if (browser_context != browser_context_) {
-    NOTREACHED();
-    return;
-  }
-
+void VpnService::DestroyConfigurationsForExtension(
+    const extensions::Extension* extension) {
   std::vector<VpnConfiguration*> to_be_destroyed;
   for (const auto& iter : key_to_configuration_map_) {
     if (iter.second->extension_id() == extension->id()) {
       to_be_destroyed.push_back(iter.second);
     }
   }
 
   for (auto& iter : to_be_destroyed) {
     DestroyConfiguration(extension->id(),             // Extension ID
                          iter->configuration_name(),  // Configuration name
                          base::Bind(base::DoNothing),
                          base::Bind(DoNothingFailureCallback));
   }
 }
 
+void VpnService::OnExtensionUninstalled(
+    content::BrowserContext* browser_context,
+    const extensions::Extension* extension,
+    extensions::UninstallReason reason) {
+  if (browser_context != browser_context_) {
+    NOTREACHED();
+    return;
+  }
+
+  DestroyConfigurationsForExtension(extension);
+}
+
 void VpnService::OnExtensionUnloaded(
     content::BrowserContext* browser_context,
     const extensions::Extension* extension,
     extensions::UnloadedExtensionInfo::Reason reason) {
   if (browser_context != browser_context_) {
     NOTREACHED();
     return;
   }
 
   if (active_configuration_ &&
       active_configuration_->extension_id() == extension->id()) {
     shill_client_->UpdateConnectionState(
         active_configuration_->object_path(),
         static_cast<uint32_t>(api_vpn::VPN_CONNECTION_STATE_FAILURE),
         base::Bind(base::DoNothing), base::Bind(DoNothingFailureCallback));
   }
+  if (extension_registry_->disabled_extensions().Contains(extension->id())) {

[pneubeck (no reviews), 2015/03/24 08:55:25]

The documentation at 
ExtensionRegistryObserver::OnExtensionUnloaded
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

is wrong. The guarantee of that function is that the extension will not be
enabled anymore.
That should be a sufficient condition to DestroyConfigurations...

I'd just call Destroy... unconditionally.

[kaliamoorthi, 2015/03/24 10:14:46]

We discussed this.

+    DestroyConfigurationsForExtension(extension);
+  }

[pneubeck (no reviews), 2015/03/24 08:55:25]

nit: remove {}

[kaliamoorthi, 2015/03/24 10:14:46]

This is not relevant anymore.

 }
 
 void VpnService::OnCreateConfigurationSuccess(
     const VpnService::SuccessCallback& callback,
     VpnConfiguration* configuration,
     const std::string& service_path) {
   configuration->set_service_path(service_path);
   service_path_to_configuration_map_[service_path] = configuration;
   shill_client_->AddShillThirdPartyVpnObserver(configuration->object_path(),
                                                configuration);
@@ skipping 59 matching lines @@
   delete configuration;
 }
 
 bool VpnService::DoesActiveConfigurationExistAndIsAccessAuthorized(
     const std::string& extension_id) {
   return active_configuration_ &&
          active_configuration_->extension_id() == extension_id;
 }
 
 }  // namespace chromeos