Create thread-safe RendererExtensionRegistry from ExtensionSet

extensions/renderer/renderer_extension_registry.h

+// Copyright 2015 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_RENDERER_RENDERER_EXTENSION_REGISTRY_H_
+#define EXTENSIONS_RENDERER_RENDERER_EXTENSION_REGISTRY_H_
+
+#include <iterator>
+#include <string>
+
+#include "base/synchronization/lock.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/extension_set.h"
+#include "url/gurl.h"
+
+namespace extensions {
+
+// Thread safe container for all loaded extensions in this process,
+// essentially the renderer counterpart to ExtensionRegistry.
+class RendererExtensionRegistry {
+ public:
+  RendererExtensionRegistry();
+  ~RendererExtensionRegistry();
+
+  static RendererExtensionRegistry* Get();
+
+  // Should only be used if using the RendererExtensionRegistry is not
+  // an option.
+  // TODO: Remove.

[not at google - send to devlin, 2015/08/18 18:42:50]

I'd reword this:

// Returns the ExtensionSet that underlies this RenderExtensionRegistry.
//
// This is not thread-safe and must only be called on the RenderThread, but
// even so, it's not thread safe because other threads may decide to
// modify this. Don't persist a reference to this.
// 
// TODO(annekao): remove or fix.

[annekao, 2015/08/18 20:21:12]

Done.

+  const ExtensionSet* GetMainThreadExtensionSet() const;

[not at google - send to devlin, 2015/08/18 18:42:50]

All my comments relating to thread safety aside, I think we can actually make
this thread safe, by giving access to the underlying ExtensionSet in a callback.
The interface would look like:

void GetMainThreadExtensionSet(
    const base::Callback<void(const ExtensionSet&)>& callback);

implemented like:

void RendererExtensionRegistry::GetMainThreadExtensionSet(callback) {
  base::AutoLock(lock_);
  callback.Run(extensions_);
}

and then call sites would be changed from things like:

bool MyFunction() {
  return OtherFunction(
      *RendererExtensionRegistry::Get()->
      GetMainThreadExtensionSet());
}

to:

void RunOtherFunction(
    bool* result, const ExtensionSet& set) {
  *result = OtherFunction(set);
}

bool MyFunction() {
  bool result;
  RendererExtensionRegistry::Get()->
     GetMainThreadExtensionSet(base::Bind(
         &RunOtherFunction, &result));
  return result;
}

quite a bit more wordy, but threadsafe.

For now I'd just make the less invasive change and go with your current
implementation of GetMainThreadExtensionSet, but I think that we should use a
threadsafe version eventually.

You might want to include in the comment, in your TODO, to make
GetMainThreadExtensionSet thead safe and callback-based.

+
+  size_t size() const;
+  bool is_empty() const;
+
+  // Forwards to the ExtensionSet methods by the same name.
+  bool Contains(const std::string& id) const;
+
+  bool Insert(const scoped_refptr<const Extension>& extension);
+
+  bool Remove(const std::string& id);
+
+  std::string GetExtensionOrAppIDByURL(const GURL& url) const;
+
+  const Extension* GetExtensionOrAppByURL(const GURL& url) const;
+
+  const Extension* GetHostedAppByURL(const GURL& url) const;
+
+  const Extension* GetByID(const std::string& id) const;
+
+  ExtensionIdSet GetIDs() const;
+
+  bool ExtensionBindingsAllowed(const GURL& url) const;
+
+ private:
+  ExtensionSet extensions_;
+
+  mutable base::Lock lock_;
+
+  DISALLOW_COPY_AND_ASSIGN(RendererExtensionRegistry);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_RENDERER_RENDERER_EXTENSION_REGISTRY_H_