[Extensions Bindings] Add support for custom property types

[Extensions Bindings] Add support for custom property types

There are a few custom types that are used as properties on extension
bindings. These are currently limited to:
- storage.StorageArea,
- types.ChromeSetting,
- types.private.ChromeDirectSetting,
- types.ContentSetting

These types are each implemented through JS custom bindings that
initialize the type and implement its functionality. Each
specifies a "js_module" in the property pointing to the custom
bindings.

Add support for these custom types in native bindings. Instead of trying
to reuse the existing JS implementation, only provide a way of
specifying a native custom type. This is for a few reasons:
- Eventually, we want all of these to be native.
- There are only four, so implementing each isn't overwhelming (as
  opposed to custom bindings in general, where are there hundreds).
- It doesn't make sense to shave this yak if we don't need to.

Add the basic infrastructure for custom types, and implement the
chrome.StorageArea type.

BUG=653596
Review-Url: https://codereview.chromium.org/2704823002
Cr-Commit-Position: refs/heads/master@{#452637}
Committed: https://chromium.googlesource.com/chromium/src/+/d31fa8e715fc2baa87357e62a106f5fce6f2d4cc

[Devlin, 2017-02-18 02:56:55]

Description was changed from

==========
storage
woot

BUG=
==========

to

==========
[Extensions Bindings] Add support for custom property types

There are a few custom types that are used as properties on extension
bindings. These are currently limited to:
- storage.StorageArea,
- types.ChromeSetting,
- types.private.ChromeDirectSetting,
- types.ContentSetting

These types are each implemented through JS custom bindings that
initialize the type and implement its functionality.

Add support for these custom types in native bindings. Instead of trying
to reuse the existing JS implementation, only provide a way of
specifying a native custom type. This is for a few reasons:
- Eventually, we want all of these to be native.
- There are only four, so implementing each isn't overwhelming (as
  opposed to custom bindings in general, where are there hundreds).
- It doesn't make sense to shave this yak if we don't need to.

Add the basic infrastructure for custom types, and implement the
chrome.StorageArea type.

BUG=653596
==========

[Devlin, 2017-02-18 02:58:42]

rdevlin.cronin@chromium.org changed reviewers:
+ jbroman@chromium.org, lazyboy@chromium.org

[Devlin, 2017-02-18 02:58:43]

Hey folks, mind taking a look?

As the CL description kind of hints at, I tried this a couple different ways,
initially targeting reusing our current js implementations (in e.g.
storage_area.js).  But it became quite a pain to try and use those, and given
that would only be a stop gap, this seemed like the better course.

https://codereview.chromium.org/2704823002/diff/20001/extensions/renderer/sto...
File extensions/renderer/storage_area.cc (right):

https://codereview.chromium.org/2704823002/diff/20001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:25: // instances, this is a little verbose.
jbroman: is there a better way to do this?  Luckily, most custom types don't
have properties, so we don't have to do this everywhere, but still kind of
annoying.

[jbroman, 2017-02-21 16:47:38]

https://codereview.chromium.org/2704823002/diff/40001/extensions/renderer/api...
File extensions/renderer/api_binding_unittest.cc (right):

https://codereview.chromium.org/2704823002/diff/40001/extensions/renderer/api...
extensions/renderer/api_binding_unittest.cc:587: EXPECT_TRUE(type_name ==
"AlphaRef" || type_name == "BetaRef") << type_name;
nit: might as well just put this in the else branch, below?

} else {
  EXPECT_TRUE(false) << type_name;
}

Not a big deal, but since you're checking anyway, it seems like it causes fewer
places to change if the list of supported properties in this case changes.

https://codereview.chromium.org/2704823002/diff/40001/extensions/renderer/api...
extensions/renderer/api_binding_unittest.cc:591: ->Set(context,
gin::StringToV8(isolate, "alphaProp"),
super-nit: it'll be internalized by V8 when you try to use it as a property key
anyways, so might as well use gin::StringToSymbol for the keys. (The perf
difference doesn't matter here, but to be consistent about using internalized
strings for keys?)

https://codereview.chromium.org/2704823002/diff/40001/extensions/renderer/sto...
File extensions/renderer/storage_area.cc (right):

https://codereview.chromium.org/2704823002/diff/40001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:151: base::Unretained(this), "get"))
The object template is per-isolate-cached, but |this| will not remain valid for
that lifetime (at best, it'll last as long as the context does -- but not even
that long, as I don't see anything preventing script from dropping the last GC
reference). You should be getting the |StorageArea| pointer from the holder,
somehow. One approach might be something like:

template <typename T>
void DecorateObjectTemplate(gin::ObjectTemplateBuilder* builder) {
  builder
      ->SetMethod("get", &T::get)
      .SetMethod("set", &T::set)
      .SetMethod("remove", &T::remove)
      .SetMethod("clear", &T::clear)
      .SetMethod("getBytesInUse", &T::getBytesInUse);
}

// and then provide implementations of all of these for each different wrappable
type

If you want to shed a little more of the gin magic (but keep
StorageArea::HandleFunctionCall more like it is now), then something like this
would work:

template <typename T>
void DecorateTemplateBuilder(gin::ObjectTemplateBuilder* builder) {
  for (const char* key : {"get", "set", "remove", "clear", "getBytesInUse"}) {
    builder->SetMethod(key, base::Bind([](const char* method_name,
gin::Arguments* arguments) {
      T* holder = nullptr;
      if (!arguments->GetHolder(&holder)) {
        arguments->ThrowTypeError("Invalid receiver for StorageArea method.");
        return;
      }
      holder->storage_area().HandleFunctionCall(method_name, arguments);
    }, key));
  }
}

[Devlin, 2017-02-21 18:05:40]

https://codereview.chromium.org/2704823002/diff/40001/extensions/renderer/api...
File extensions/renderer/api_binding_unittest.cc (right):

https://codereview.chromium.org/2704823002/diff/40001/extensions/renderer/api...
extensions/renderer/api_binding_unittest.cc:587: EXPECT_TRUE(type_name ==
"AlphaRef" || type_name == "BetaRef") << type_name;
On 2017/02/21 16:47:37, jbroman wrote:
> nit: might as well just put this in the else branch, below?
> 
> } else {
>   EXPECT_TRUE(false) << type_name;
> }
> 
> Not a big deal, but since you're checking anyway, it seems like it causes
fewer
> places to change if the list of supported properties in this case changes.

Done.

https://codereview.chromium.org/2704823002/diff/40001/extensions/renderer/api...
extensions/renderer/api_binding_unittest.cc:591: ->Set(context,
gin::StringToV8(isolate, "alphaProp"),
On 2017/02/21 16:47:37, jbroman wrote:
> super-nit: it'll be internalized by V8 when you try to use it as a property
key
> anyways, so might as well use gin::StringToSymbol for the keys. (The perf
> difference doesn't matter here, but to be consistent about using internalized
> strings for keys?)

Done.

https://codereview.chromium.org/2704823002/diff/40001/extensions/renderer/sto...
File extensions/renderer/storage_area.cc (right):

https://codereview.chromium.org/2704823002/diff/40001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:151: base::Unretained(this), "get"))
On 2017/02/21 16:47:37, jbroman wrote:
> The object template is per-isolate-cached, but |this| will not remain valid
for
> that lifetime (at best, it'll last as long as the context does -- but not even
> that long, as I don't see anything preventing script from dropping the last GC
> reference). You should be getting the |StorageArea| pointer from the holder,
> somehow. One approach might be something like:
> 
> template <typename T>
> void DecorateObjectTemplate(gin::ObjectTemplateBuilder* builder) {
>   builder
>       ->SetMethod("get", &T::get)
>       .SetMethod("set", &T::set)
>       .SetMethod("remove", &T::remove)
>       .SetMethod("clear", &T::clear)
>       .SetMethod("getBytesInUse", &T::getBytesInUse);
> }
> 
> // and then provide implementations of all of these for each different
wrappable
> type
> 
> If you want to shed a little more of the gin magic (but keep
> StorageArea::HandleFunctionCall more like it is now), then something like this
> would work:
> 
> template <typename T>
> void DecorateTemplateBuilder(gin::ObjectTemplateBuilder* builder) {
>   for (const char* key : {"get", "set", "remove", "clear", "getBytesInUse"}) {
>     builder->SetMethod(key, base::Bind([](const char* method_name,
> gin::Arguments* arguments) {
>       T* holder = nullptr;
>       if (!arguments->GetHolder(&holder)) {
>         arguments->ThrowTypeError("Invalid receiver for StorageArea method.");
>         return;
>       }
>       holder->storage_area().HandleFunctionCall(method_name, arguments);
>     }, key));
>   }
> }

Opted for the second - I'm really not keen on re-implementing each of these
three times when the only thing that really needs to change is the value of the
properties (and none of the implementation).

[jbroman, 2017-02-21 20:22:07]

lgtm

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
File extensions/renderer/storage_area.cc (right):

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:173: v8::Local<v8::Context> context =
isolate->GetCurrentContext();
nit: I think we've been using holder context elsewhere for non-static functions
(as Blink does)?

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:238: auto raw_signature = signature.get();
nit: use auto* (or for this use, even "const auto*") when deducing pointer types

[lazyboy, 2017-02-22 03:37:03]

Some comments/questions.

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
File extensions/renderer/api_binding.cc (right):

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
extensions/renderer/api_binding.cc:119: std::string type_name;
I find giving examples of these are helpful to navigate the code.
e.g. chrome.storage.local refers to type StorageArea, type_name would be
"storage.StorageArea", property_name would be "local"

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
extensions/renderer/api_binding.cc:306: if (dict->GetString("$ref", &ref)) {
For storage, we would get "StorageArea" as $ref in JSON and since we registered
the custom type "storage.StorageArea" in native_extensions_bindings_system.cc,
we will be able to retrieve the custom type?

So the custom bindings need to be implemented in c++ but we need to express its
presence in JSON schema, right? I'd document that in
APIBindingsSystem::RegisterCustomType()

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
File extensions/renderer/api_bindings_system.cc (right):

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
extensions/renderer/api_bindings_system.cc:122:
DCHECK(custom_types_.find(type_name) == custom_types_.end());
nit: << "Custom type already registered, type_name: " << type_name;

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
extensions/renderer/api_bindings_system.cc:131: DCHECK(iter !=
custom_types_.end());
nit: << "Custom type not found: " << type_name

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
File extensions/renderer/storage_area.cc (right):

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:34: for (const char* key : {"get", "set",
"remove", "clear", "getBytesInUse"})
These comes from "js_module" key in current JS bindings, right? I'd mention
"js_module" in the CL description.

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:55: builder.SetValue("QUOTA_BYTES",
api::storage::local::QUOTA_BYTES);
What's the plan for documentation generation for these as they won't be coming
from json schema anymore?

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:59: StorageArea& storage_area() { return
storage_area_; }
optional: Should storage_area_ be part of a common base class for all
XXXStorageArea classes?

[Devlin, 2017-02-22 20:40:20]

Description was changed from

==========
[Extensions Bindings] Add support for custom property types

There are a few custom types that are used as properties on extension
bindings. These are currently limited to:
- storage.StorageArea,
- types.ChromeSetting,
- types.private.ChromeDirectSetting,
- types.ContentSetting

These types are each implemented through JS custom bindings that
initialize the type and implement its functionality.

Add support for these custom types in native bindings. Instead of trying
to reuse the existing JS implementation, only provide a way of
specifying a native custom type. This is for a few reasons:
- Eventually, we want all of these to be native.
- There are only four, so implementing each isn't overwhelming (as
  opposed to custom bindings in general, where are there hundreds).
- It doesn't make sense to shave this yak if we don't need to.

Add the basic infrastructure for custom types, and implement the
chrome.StorageArea type.

BUG=653596
==========

to

==========
[Extensions Bindings] Add support for custom property types

There are a few custom types that are used as properties on extension
bindings. These are currently limited to:
- storage.StorageArea,
- types.ChromeSetting,
- types.private.ChromeDirectSetting,
- types.ContentSetting

These types are each implemented through JS custom bindings that
initialize the type and implement its functionality. Each
specifies a "jsmodule" in the property pointing to the custom
bindings.

Add support for these custom types in native bindings. Instead of trying
to reuse the existing JS implementation, only provide a way of
specifying a native custom type. This is for a few reasons:
- Eventually, we want all of these to be native.
- There are only four, so implementing each isn't overwhelming (as
  opposed to custom bindings in general, where are there hundreds).
- It doesn't make sense to shave this yak if we don't need to.

Add the basic infrastructure for custom types, and implement the
chrome.StorageArea type.

BUG=653596
==========

[Devlin, 2017-02-22 20:55:18]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 20:55:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Devlin, 2017-02-22 20:56:45]

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
File extensions/renderer/api_binding.cc (right):

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
extensions/renderer/api_binding.cc:119: std::string type_name;
On 2017/02/22 03:37:03, lazyboy wrote:
> I find giving examples of these are helpful to navigate the code.
> e.g. chrome.storage.local refers to type StorageArea, type_name would be
> "storage.StorageArea", property_name would be "local"

Done.

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
extensions/renderer/api_binding.cc:306: if (dict->GetString("$ref", &ref)) {
On 2017/02/22 03:37:03, lazyboy wrote:
> For storage, we would get "StorageArea" as $ref in JSON and since we
registered
> the custom type "storage.StorageArea" in native_extensions_bindings_system.cc,
> we will be able to retrieve the custom type?

In our json generation/compression, we fully specify all types, so the ref
becomes storage.StorageArea.  Check out
src/out/Debug/gen/extensions/common/api/generated_schemas.cc.

> So the custom bindings need to be implemented in c++ but we need to express
its
> presence in JSON schema, right? I'd document that in
> APIBindingsSystem::RegisterCustomType()

Sg; done.

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
File extensions/renderer/api_bindings_system.cc (right):

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
extensions/renderer/api_bindings_system.cc:122:
DCHECK(custom_types_.find(type_name) == custom_types_.end());
On 2017/02/22 03:37:03, lazyboy wrote:
> nit: << "Custom type already registered, type_name: " << type_name;

Done.

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/api...
extensions/renderer/api_bindings_system.cc:131: DCHECK(iter !=
custom_types_.end());
On 2017/02/22 03:37:03, lazyboy wrote:
> nit: << "Custom type not found: " << type_name

Done.

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
File extensions/renderer/storage_area.cc (right):

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:34: for (const char* key : {"get", "set",
"remove", "clear", "getBytesInUse"})
On 2017/02/22 03:37:03, lazyboy wrote:
> These comes from "js_module" key in current JS bindings, right? I'd mention
> "js_module" in the CL description.

Done.

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:55: builder.SetValue("QUOTA_BYTES",
api::storage::local::QUOTA_BYTES);
On 2017/02/22 03:37:03, lazyboy wrote:
> What's the plan for documentation generation for these as they won't be coming
> from json schema anymore?

Right now, they're still half-coming-from-the-json, in that
api::storage::local::FOO is a generated file from the json, and that's what
we're using.

I thought about passing around the properties dictionary and using that, but
StorageArea is, AFAICT, the *only* implementation that does this.  So I figured
that it was better to have explicit code than create more spaghetti for the
one-off case.

I'm hoping that since we still link for the values, people will update both at
the same time (because adding another property for "FOO", <magic_number> would
look weird, and to add the constant to api::storage::local::FOO, you have to add
it to the json, and thus the documentation).

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:59: StorageArea& storage_area() { return
storage_area_; }
On 2017/02/22 03:37:03, lazyboy wrote:
> optional: Should storage_area_ be part of a common base class for all
> XXXStorageArea classes?

Unfortunately, we can't.  gin::Wrappable<> does not allow for further
inheritance (i.e., we don't allow Foo : gin::Wrappable<>, CustomFoo : Foo), and
by style we only allow a single non-pure-interface inheritance (so having a Foo,
CustomFoo : Foo, gin::Wrappable<> is also out).

This was frustrating, because it *is* a lot of extra code, but I couldn't think
of a better way.

On the upside, we only need these for the cases where there are different
properties/methods for the different objects.

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:173: v8::Local<v8::Context> context =
isolate->GetCurrentContext();
On 2017/02/21 20:22:07, jbroman wrote:
> nit: I think we've been using holder context elsewhere for non-static
functions
> (as Blink does)?

Ah, yes, we have.  It's a little messier here, because in order to get at it we
need to do:
v8::Local<v8::Object> holder;
arguments->GetHolder(&holder);
holder->CreationContext()

Which isn't terrible, but not quite as clean.  In a followup, what do you think
of exposing something on gin::Arguments?  Either a Holder() that returns a
v8::Value<> or an info accessor to get at the raw v8::FunctionCallbackInfo.

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:238: auto raw_signature = signature.get();
On 2017/02/21 20:22:07, jbroman wrote:
> nit: use auto* (or for this use, even "const auto*") when deducing pointer
types

Done.

[jbroman, 2017-02-22 21:05:28]

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
File extensions/renderer/storage_area.cc (right):

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:173: v8::Local<v8::Context> context =
isolate->GetCurrentContext();
On 2017/02/22 at 20:56:45, Devlin wrote:
> On 2017/02/21 20:22:07, jbroman wrote:
> > nit: I think we've been using holder context elsewhere for non-static
functions
> > (as Blink does)?
> 
> Ah, yes, we have.  It's a little messier here, because in order to get at it
we need to do:
> v8::Local<v8::Object> holder;
> arguments->GetHolder(&holder);
> holder->CreationContext()
> 
> Which isn't terrible, but not quite as clean.  In a followup, what do you
think of exposing something on gin::Arguments?  Either a Holder() that returns a
v8::Value<> or an info accessor to get at the raw v8::FunctionCallbackInfo.

I have no objection to something along those lines, but I don't feel strongly.

[lazyboy, 2017-02-22 22:15:24]

s/"jsmodule"/"js_module" in CL description.

lgtm.

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
File extensions/renderer/storage_area.cc (right):

https://codereview.chromium.org/2704823002/diff/60001/extensions/renderer/sto...
extensions/renderer/storage_area.cc:55: builder.SetValue("QUOTA_BYTES",
api::storage::local::QUOTA_BYTES);
On 2017/02/22 20:56:45, Devlin wrote:
> On 2017/02/22 03:37:03, lazyboy wrote:
> > What's the plan for documentation generation for these as they won't be
coming
> > from json schema anymore?
> 
> Right now, they're still half-coming-from-the-json, in that
> api::storage::local::FOO is a generated file from the json, and that's what
> we're using.
> 
> I thought about passing around the properties dictionary and using that, but
> StorageArea is, AFAICT, the *only* implementation that does this.  So I
figured
> that it was better to have explicit code than create more spaghetti for the
> one-off case.
Yes. Agreed.
> 
> I'm hoping that since we still link for the values, people will update both at
> the same time (because adding another property for "FOO", <magic_number> would
> look weird, and to add the constant to api::storage::local::FOO, you have to
add
> it to the json, and thus the documentation).

Acknowledged.

[Devlin, 2017-02-22 22:17:49]

Description was changed from

==========
[Extensions Bindings] Add support for custom property types

There are a few custom types that are used as properties on extension
bindings. These are currently limited to:
- storage.StorageArea,
- types.ChromeSetting,
- types.private.ChromeDirectSetting,
- types.ContentSetting

These types are each implemented through JS custom bindings that
initialize the type and implement its functionality. Each
specifies a "jsmodule" in the property pointing to the custom
bindings.

Add support for these custom types in native bindings. Instead of trying
to reuse the existing JS implementation, only provide a way of
specifying a native custom type. This is for a few reasons:
- Eventually, we want all of these to be native.
- There are only four, so implementing each isn't overwhelming (as
  opposed to custom bindings in general, where are there hundreds).
- It doesn't make sense to shave this yak if we don't need to.

Add the basic infrastructure for custom types, and implement the
chrome.StorageArea type.

BUG=653596
==========

to

==========
[Extensions Bindings] Add support for custom property types

There are a few custom types that are used as properties on extension
bindings. These are currently limited to:
- storage.StorageArea,
- types.ChromeSetting,
- types.private.ChromeDirectSetting,
- types.ContentSetting

These types are each implemented through JS custom bindings that
initialize the type and implement its functionality. Each
specifies a "js_module" in the property pointing to the custom
bindings.

Add support for these custom types in native bindings. Instead of trying
to reuse the existing JS implementation, only provide a way of
specifying a native custom type. This is for a few reasons:
- Eventually, we want all of these to be native.
- There are only four, so implementing each isn't overwhelming (as
  opposed to custom bindings in general, where are there hundreds).
- It doesn't make sense to shave this yak if we don't need to.

Add the basic infrastructure for custom types, and implement the
chrome.StorageArea type.

BUG=653596
==========

[Devlin, 2017-02-22 22:18:05]

On 2017/02/22 22:15:24, lazyboy wrote:
> s/"jsmodule"/"js_module" in CL description.
Whoops, done.

[Devlin, 2017-02-22 22:18:10]

The CQ bit was unchecked by rdevlin.cronin@chromium.org

[Devlin, 2017-02-22 22:18:14]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2017-02-22 22:18:14]

The patchset sent to the CQ was uploaded after l-g-t-m from jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2704823002/#ps80001
(title: "jbroman + lazyboy")

[commit-bot: I haz the power, 2017-02-22 22:18:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Devlin, 2017-02-23 00:41:51]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 00:42:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 04:15:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 04:15:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Devlin, 2017-02-23 19:04:32]

Patchset #7 (id:110001) has been deleted

[Devlin, 2017-02-23 19:21:40]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 19:22:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 20:38:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 20:38:34]

Dry run: This issue passed the CQ dry run.

[Devlin, 2017-02-23 21:28:18]

FYI, had to update the StorageArea implementation to not use base::Bind()'d
methods in SetMethod.  See cbrug.com/695618.

[Devlin, 2017-02-23 21:29:48]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2017-02-23 21:29:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
lazyboy@chromium.org, jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2704823002/#ps120001
(title: "asan fix")

[commit-bot: I haz the power, 2017-02-23 21:30:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 21:36:59]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1487885388425900,
"parent_rev": "100accb9fb85dc4a5fd736f972993d6706b2566e", "commit_rev":
"d31fa8e715fc2baa87357e62a106f5fce6f2d4cc"}

[commit-bot: I haz the power, 2017-02-23 21:37:38]

Description was changed from

==========
[Extensions Bindings] Add support for custom property types

There are a few custom types that are used as properties on extension
bindings. These are currently limited to:
- storage.StorageArea,
- types.ChromeSetting,
- types.private.ChromeDirectSetting,
- types.ContentSetting

These types are each implemented through JS custom bindings that
initialize the type and implement its functionality. Each
specifies a "js_module" in the property pointing to the custom
bindings.

Add support for these custom types in native bindings. Instead of trying
to reuse the existing JS implementation, only provide a way of
specifying a native custom type. This is for a few reasons:
- Eventually, we want all of these to be native.
- There are only four, so implementing each isn't overwhelming (as
  opposed to custom bindings in general, where are there hundreds).
- It doesn't make sense to shave this yak if we don't need to.

Add the basic infrastructure for custom types, and implement the
chrome.StorageArea type.

BUG=653596
==========

to

==========
[Extensions Bindings] Add support for custom property types

There are a few custom types that are used as properties on extension
bindings. These are currently limited to:
- storage.StorageArea,
- types.ChromeSetting,
- types.private.ChromeDirectSetting,
- types.ContentSetting

These types are each implemented through JS custom bindings that
initialize the type and implement its functionality. Each
specifies a "js_module" in the property pointing to the custom
bindings.

Add support for these custom types in native bindings. Instead of trying
to reuse the existing JS implementation, only provide a way of
specifying a native custom type. This is for a few reasons:
- Eventually, we want all of these to be native.
- There are only four, so implementing each isn't overwhelming (as
  opposed to custom bindings in general, where are there hundreds).
- It doesn't make sense to shave this yak if we don't need to.

Add the basic infrastructure for custom types, and implement the
chrome.StorageArea type.

BUG=653596

Review-Url: https://codereview.chromium.org/2704823002
Cr-Commit-Position: refs/heads/master@{#452637}
Committed:
https://chromium.googlesource.com/chromium/src/+/d31fa8e715fc2baa87357e62a106...
==========

[commit-bot: I haz the power, 2017-02-23 21:37:39]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/d31fa8e715fc2baa87357e62a106...