[Extensions Bindings] Add ChromeSetting custom type

extensions/renderer/chrome_setting.h

Index: extensions/renderer/chrome_setting.h
diff --git a/extensions/renderer/chrome_setting.h b/extensions/renderer/chrome_setting.h
new file mode 100644
index 0000000000000000000000000000000000000000..47ea1b735d46763c1e6a34bf0e7acfbbfbbadbf2
--- /dev/null
+++ b/extensions/renderer/chrome_setting.h
@@ -0,0 +1,86 @@
+// Copyright 2017 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_CHROME_SETTING_H_
+#define EXTENSIONS_RENDERER_CHROME_SETTING_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "extensions/renderer/argument_spec.h"
+#include "gin/wrappable.h"
+#include "v8/include/v8.h"
+
+namespace base {
+class DictionaryValue;
+}
+
+namespace gin {
+class Arguments;
+}
+
+namespace extensions {
+class APIEventHandler;
+class APIRequestHandler;
+
+// The custom implementation of the ChromeSetting type exposed to APIs.
+class ChromeSetting final : public gin::Wrappable<ChromeSetting> {
+ public:
+  ~ChromeSetting() override;
+
+  // Creates a ChromeSetting object for the given property.
+  static v8::Local<v8::Object> CreateChromeSetting(

[jbroman, 2017/02/24 18:08:45]

nit: could just call it "Create", given it will be invoked as
ChromeSetting::CreateChromeSetting?

[Devlin, 2017/02/24 20:12:28]

Done.

+      v8::Local<v8::Context> context,
+      const std::string& property_name,
+      APIRequestHandler* request_handler,
+      APIEventHandler* event_handler,
+      APITypeReferenceMap* type_refs);
+
+  static gin::WrapperInfo kWrapperInfo;
+
+  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
+      v8::Isolate* isolate) override;
+
+ private:
+  ChromeSetting(APIRequestHandler* request_handler,
+                APIEventHandler* event_handler,
+                const APITypeReferenceMap* type_refs,
+                const std::string& pref_name,
+                const base::DictionaryValue& argument_spec);
+
+  // JS function handlers:
+  void Get(gin::Arguments* arguments);
+  void Set(gin::Arguments* arguments);
+  void Clear(gin::Arguments* arguments);
+
+  // Returns the onChange event associated with the ChromeSetting.
+  v8::Local<v8::Value> GetOnChangeEvent(gin::Arguments* arguments);
+
+  // Common function handling endpoint.
+  void HandleFunction(const std::string& function_name,
+                      gin::Arguments* arguments);
+
+  APIRequestHandler* request_handler_;
+
+  APIEventHandler* event_handler_;
+
+  const APITypeReferenceMap* type_refs_;
+
+  // The name of the preference this ChromeSetting is managing.
+  std::string pref_name_;
+
+  // The type of argument that calling set() on the ChromeSetting expects (since
+  // different settings can take a different type of argument depending on the
+  // preference it manages).
+  ArgumentSpec argument_spec_;
+
+  // A reference to the onChange event.
+  v8::Global<v8::Value> event_;

[jbroman, 2017/02/24 18:08:45]

This can cause a leak. Suppose the extension author does this:

var setting; // some ChromeSetting
setting.onChange.foo = setting;

Or otherwise causes some path from onChange back to the setting. Now there's a
reference cycle, and one of the elements of the cycle is a global persistent
handle, so it will not get collected (and will keep both objects and the
creation context alive forever).

One way to resolve this would be to have the event object be held via a private
property (using v8::Private, like in APIBindingBridge), rather than a persistent
handle. Then the cycle would exist in the V8 heap (with no persistent handles),
and thus be collectible.

[Devlin, 2017/02/24 20:12:28]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(ChromeSetting);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_RENDERER_CHROME_SETTING_H_