Remove RuntimeAPIDelegate::GetPreviousVersion.

extensions/browser/extension_registry.h

 // Copyright 2013 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.
 
 #ifndef EXTENSIONS_BROWSER_EXTENSION_REGISTRY_H_
 #define EXTENSIONS_BROWSER_EXTENSION_REGISTRY_H_
 
 #include <string>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "extensions/browser/uninstall_reason.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/features/features.h"
 
 #if !BUILDFLAG(ENABLE_EXTENSIONS)
 #error "Extensions must be enabled"
 #endif
 
+namespace base {
+class Version;

[Devlin, 2017/05/19 02:04:01]

nit: Since we return a non-pointer for base::Version, don't we need to include
version.h?  (I'm suspecting it's transitively included right now)

[lazyboy, 2017/05/19 18:24:55]

Yes, fixed.

+}
+
 namespace content {
 class BrowserContext;
 }
 
 namespace extensions {
 class Extension;
 class ExtensionRegistryObserver;
 
 // ExtensionRegistry holds sets of the installed extensions for a given
 // BrowserContext. An incognito browser context and its master browser context
@@ skipping 42 matching lines @@
 
   // Returns a set of all extensions in the subsets specified by |include_mask|.
   //  * enabled_extensions()     --> ExtensionRegistry::ENABLED
   //  * disabled_extensions()    --> ExtensionRegistry::DISABLED
   //  * terminated_extensions()  --> ExtensionRegistry::TERMINATED
   //  * blacklisted_extensions() --> ExtensionRegistry::BLACKLISTED
   //  * blocked_extensions()     --> ExtensionRegistry::BLOCKED
   std::unique_ptr<ExtensionSet> GenerateInstalledExtensionsSet(
       int include_mask) const;
 
+  // Returns the current version of the extension.
+  // Note that if we are in the process of updating the extension, this will
+  // return the version before update.
+  //
+  // TODO(lazyboy): Consider updating callers to directly retrieve version()
+  // from either GetExtensionById() or querying ExtensionSet getters of this
+  // class.
+  base::Version GetVersion(const Extension* extension) const;

[Devlin, 2017/05/19 02:04:00]

Without the context of this patch, it's *very* why this is different than
calling Extension::version() on the passed-in extension.

What if do something like:
// Returns the current version of the extension with the given |id|, if
// one exists.
// Note: If we are currently updating the extension, this returns the
// version stored currently, rather than the in-progress update.
base::Version GetStoredVersion(const ExtensionId& id);

WDYT?

[lazyboy, 2017/05/19 18:24:55]

GetStoredVersion is definitely better than GetVersion, which would have kept
readers guessing, thanks!

+
   // The usual observer interface.
   void AddObserver(ExtensionRegistryObserver* observer);
   void RemoveObserver(ExtensionRegistryObserver* observer);
 
   // Invokes the observer method OnExtensionLoaded(). The extension must be
   // enabled at the time of the call.
   void TriggerOnLoaded(const Extension* extension);
 
   // Invokes the observer method OnExtensionReady(). This always follows
   // an OnLoaded event, but is not called until it's safe to create the
@@ skipping 101 matching lines @@
   base::ObserverList<ExtensionRegistryObserver> observers_;
 
   content::BrowserContext* const browser_context_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionRegistry);
 };
 
 }  // namespace extensions
 
 #endif  // EXTENSIONS_BROWSER_EXTENSION_REGISTRY_H_