Need v8 bindings for DOM Storage

[Jeremy Orlow]

Need v8 bindings for DOM Storage (for Chromium and Android).

[Jeremy Orlow]

Chromium side being tracked at http://crbug.com/14006

[Jeremy Orlow]

Created attachment 31214 [details]
v1

Create custom bindings for v8.  The rest of these files are still forked (so the review is happening on the chromium review site).  These bindings have been tested on a hacked up Chromium instance (also running --single-process) and Android.

[Dimitri Glazkov (Google)]

Comment on attachment 31214 [details]
v1

> [phone]  jorlow  <[e-mail]>

Need to set your environment variables: http://webkit.org/coding/contributing.html

> +        * bindings/v8/custom/V8StorageCustom.cpp: Added.

When you add a file, 

> +        (WebCore::V8Custom::v8StorageNamedPropertyEnumerator):
> +        (WebCore::storageGetter):
> +        (WebCore::INDEXED_PROPERTY_GETTER):
> +        (WebCore::NAMED_PROPERTY_GETTER):
> +        (WebCore::storageSetter):
> +        (WebCore::INDEXED_PROPERTY_SETTER):
> +        (WebCore::NAMED_PROPERTY_SETTER):
> +        (WebCore::storageDeleter):
> +        (WebCore::INDEXED_PROPERTY_DELETER):
> +        (WebCore::NAMED_PROPERTY_DELETER):

... there's no need to leave all the members listed in ChangeLog.

> +#include "config.h"
> +
> +#include "v8_binding.h"
V8Binding.h

> +#include "v8_custom.h"
V8CustomBinding.h

> +#include "V8Proxy.h"
> +
> +#include "Storage.h"

headers are out of order.

> +static v8::Handle<v8::Value> storageSetter(v8::Local<v8::String> nameV8, v8::Local<v8::Value> valueV8, const v8::AccessorInfo& info)

The convention is prefixes, rather than suffixes: v8Name, v8Value

> +
> +NAMED_PROPERTY_DELETER(Storage) {

Brace on new line.

> +    INC_STATS("DOM.Storage.NamedPropertyDeleter");
> +    return storageDeleter(name, info);
> +}

[Jeremy Orlow]

Created attachment 31215 [details]
v2

Fixed Dimitri's issues.

[Dimitri Glazkov (Google)]

Comment on attachment 31215 [details]
v2

> +    String name = ToWebCoreString(v8Name);
> +    String value = ToWebCoreString(v8Value);

toWebCoreString, sorry -- should've noticed earlier.

> +    String name = ToWebCoreString(v8Name);

ditto.

[Jeremy Orlow]

Created attachment 31216 [details]
v3

[David Levin]

Comment on attachment 31216 [details]
v3

         ASSERT(ec == 0);
should be (avoid ==, != comparisons to 0 in WK code).
         ASSERT(!ec);
but this can be done on landing.

[David Levin]

Assigned to levin for landing.

[Dmitry Titov]

Landed: http://trac.webkit.org/changeset/44636

[Jeremy Orlow]

Things moved around a bit since I started this patch.  Need to upstream another file.

[Jeremy Orlow]

Created attachment 31329 [details]
part 2 - v1

Define the functions in V8CustomBindings.h.  "Forgot" this in my earlier patch since it wasn't fully upstreamed when I wrote the patch.

[Jeremy Orlow]

Created attachment 31330 [details]
part2 - v2

Forgot to set my name.  :-)

[Dimitri Glazkov (Google)]

Comment on attachment 31330 [details]
part2 - v2

I'll land.

[Dimitri Glazkov (Google)]

Fix landed as http://trac.webkit.org/changeset/44706.