WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
26356
Need v8 bindings for DOM Storage
https://bugs.webkit.org/show_bug.cgi?id=26356
Summary
Need v8 bindings for DOM Storage
Jeremy Orlow
Reported
2009-06-12 14:35:25 PDT
Need v8 bindings for DOM Storage (for Chromium and Android).
Attachments
v1
(6.54 KB, patch)
2009-06-12 14:48 PDT
,
Jeremy Orlow
dglazkov
: review-
Details
Formatted Diff
Diff
v2
(5.95 KB, patch)
2009-06-12 15:17 PDT
,
Jeremy Orlow
dglazkov
: review-
Details
Formatted Diff
Diff
v3
(5.95 KB, patch)
2009-06-12 15:24 PDT
,
Jeremy Orlow
levin
: review+
Details
Formatted Diff
Diff
part 2 - v1
(1.55 KB, patch)
2009-06-15 20:47 PDT
,
Jeremy Orlow
no flags
Details
Formatted Diff
Diff
part2 - v2
(1.54 KB, patch)
2009-06-15 20:51 PDT
,
Jeremy Orlow
dglazkov
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Jeremy Orlow
Comment 1
2009-06-12 14:42:42 PDT
Chromium side being tracked at
http://crbug.com/14006
Jeremy Orlow
Comment 2
2009-06-12 14:48:49 PDT
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 3
2009-06-12 15:01:44 PDT
Comment on
attachment 31214
[details]
v1
> +2009-06-12 jorlow <
jorlow@chromium.org
>
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
Comment 4
2009-06-12 15:17:35 PDT
Created
attachment 31215
[details]
v2 Fixed Dimitri's issues.
Dimitri Glazkov (Google)
Comment 5
2009-06-12 15:20:32 PDT
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
Comment 6
2009-06-12 15:24:09 PDT
Created
attachment 31216
[details]
v3
David Levin
Comment 7
2009-06-12 15:46:20 PDT
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
Comment 8
2009-06-12 15:48:13 PDT
Assigned to levin for landing.
Dmitry Titov
Comment 9
2009-06-12 16:21:53 PDT
Landed:
http://trac.webkit.org/changeset/44636
Jeremy Orlow
Comment 10
2009-06-15 20:33:01 PDT
Things moved around a bit since I started this patch. Need to upstream another file.
Jeremy Orlow
Comment 11
2009-06-15 20:47:06 PDT
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
Comment 12
2009-06-15 20:51:37 PDT
Created
attachment 31330
[details]
part2 - v2 Forgot to set my name. :-)
Dimitri Glazkov (Google)
Comment 13
2009-06-15 20:52:45 PDT
Comment on
attachment 31330
[details]
part2 - v2 I'll land.
Dimitri Glazkov (Google)
Comment 14
2009-06-15 21:00:38 PDT
Fix landed as
http://trac.webkit.org/changeset/44706
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug