Create thread-safe RendererExtensionRegistry from ExtensionSet

extensions/renderer/renderer_extension_registry.h

 // Copyright 2013 The Chromium Authors. All rights reserved.

[not at google - send to devlin, 2015/08/17 20:43:05]

2015, and in the .cc file.

[annekao, 2015/08/17 23:51:10]

Done.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef EXTENSIONS_COMMON_EXTENSION_SET_H_
-#define EXTENSIONS_COMMON_EXTENSION_SET_H_
+#ifndef EXTENSIONS_RENDERER_RENDERER_EXTENSION_REGISTRY_H_
+#define EXTENSIONS_RENDERER_RENDERER_EXTENSION_REGISTRY_H_
 
 #include <iterator>
 #include <map>

[not at google - send to devlin, 2015/08/17 20:43:05]

you can remove this <map> include.

[annekao, 2015/08/17 23:51:10]

Done.

 #include <string>
 
 #include "base/callback.h"

[not at google - send to devlin, 2015/08/17 20:43:05]

and once you delete that modification method, you can delete this include as
well.

[annekao, 2015/08/17 23:51:10]

Done.

-#include "base/gtest_prod_util.h"
-#include "base/memory/ref_counted.h"
+#include "base/synchronization/lock.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/extension_set.h"
 #include "url/gurl.h"
 
 namespace extensions {
 
-// The one true extension container. Extensions are identified by their id.
+// Thread-safe extension container.  Contains all loaded extensions.
+// This is essentially the renderer counterpart to ExtensionService in
+// the browser. It contains information about all extensions currently
+// loaded by the browser.  All functions forward to ExtensionSet.
+// Extensions are identified by their id.

[not at google - send to devlin, 2015/08/17 20:43:05]

Let's trim down this comment as well, it's unnecessarily wordy (and you usually
don't need to include details about a class' implementation details in its class
comment):

// Thread safe container for all loaded extensions in this process,
// essentially the renderer counterpart to ExtensionRegistry.

[annekao, 2015/08/17 23:51:10]

Done.

 // Only one extension can be in the set with a given ID.
-class ExtensionSet {
+class RendererExtensionRegistry {
  public:
-  typedef std::pair<base::FilePath, std::string> ExtensionPathAndDefaultLocale;
-  typedef std::map<std::string, scoped_refptr<const Extension> > ExtensionMap;
-  typedef base::Callback<void(const ExtensionIdSet&)>
-      ModificationCallback;
+  RendererExtensionRegistry();
+  ~RendererExtensionRegistry();
 
-  // Iteration over the values of the map (given that it's an ExtensionSet,
-  // it should iterate like a set iterator).
-  class const_iterator : public std::iterator<std::input_iterator_tag,
-                                              scoped_refptr<const Extension> > {
-   public:
-    const_iterator();
-    const_iterator(const const_iterator& other);
-    explicit const_iterator(ExtensionMap::const_iterator it);
-    ~const_iterator();
-    const_iterator& operator++() {
-      ++it_;
-      return *this;
-    }
-    const_iterator operator++(int) {
-      const const_iterator old(*this);
-      ++it_;
-      return old;
-    }
-    const scoped_refptr<const Extension>& operator*() const {
-      return it_->second;
-    }
-    const scoped_refptr<const Extension>* operator->() const {
-      return &it_->second;
-    }
-    bool operator!=(const const_iterator& other) const {
-      return it_ != other.it_;
-    }
-    bool operator==(const const_iterator& other) const {
-      return it_ == other.it_;
-    }
+  static RendererExtensionRegistry* GetRegistry();

[not at google - send to devlin, 2015/08/17 20:43:05]

Typically singleton accessors are just "Get" not "GetRegistry"... no need to
repeat "registry" when calling RendererExtensionRegistry::GetRegistry.

[annekao, 2015/08/17 23:51:10]

Done.

 
-   private:
-    ExtensionMap::const_iterator it_;
-  };
-
-  ExtensionSet();
-  ~ExtensionSet();
+  ExtensionSet* GetMainThreadExtensionSet();
 
   size_t size() const;
   bool is_empty() const;
 
   // Iteration support.
-  const_iterator begin() const { return const_iterator(extensions_.begin()); }
-  const_iterator end() const { return const_iterator(extensions_.end()); }
+  ExtensionSet::const_iterator begin() const { return extensions_.begin(); }
+  ExtensionSet::const_iterator end() const { return extensions_.end(); }
 
   // Returns true if the set contains the specified extension.
   bool Contains(const std::string& id) const;
 
   // Adds the specified extension to the set. The set becomes an owner. Any
   // previous extension with the same ID is removed.
   // Returns true if there is no previous extension.
   bool Insert(const scoped_refptr<const Extension>& extension);
 
   // Copies different items from |extensions| to the current set and returns
@@ skipping 40 matching lines @@
 
   // Gets the IDs of all extensions in the set.
   ExtensionIdSet GetIDs() const;
 
   // Returns true if |info| should get extension api bindings and be permitted
   // to make api calls. Note that this is independent of what extension
   // permissions the given extension has been granted.
   bool ExtensionBindingsAllowed(const GURL& url) const;
 
   void set_modification_callback(
-      const ModificationCallback& modification_callback) {
+      const ExtensionSet::ModificationCallback& modification_callback) {
     modification_callback_ = modification_callback;
   }
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(ExtensionSetTest, ExtensionSet);
+  ExtensionSet extensions_;
 
-  ExtensionMap extensions_;
+  mutable base::Lock lock_;
 
   // If non-null, called with the extension ids in this set after a modification
   // occurred. This is not called on Clear() which is typically used when
   // discarding the set (e.g., on shutdown) and we do not want to track that as
   // a real modification.
-  ModificationCallback modification_callback_;
+  ExtensionSet::ModificationCallback modification_callback_;

[not at google - send to devlin, 2015/08/17 20:43:05]

Let's trim this interface a bit. Can we get rid of the modification callback at
least?

[annekao, 2015/08/17 23:51:10]

Done. Took out functions that weren't used in any of the renderer files.

 
-  DISALLOW_COPY_AND_ASSIGN(ExtensionSet);
+  DISALLOW_COPY_AND_ASSIGN(RendererExtensionRegistry);
 };
 
 }  // namespace extensions
 
-#endif  // EXTENSIONS_COMMON_EXTENSION_SET_H_
+#endif  // EXTENSIONS_RENDERER_RENDERER_EXTENSION_REGISTRY_H_