Bug 33985 - The Chromium WebKit API needs to expose storage event related data
Summary: The Chromium WebKit API needs to expose storage event related data
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Jeremy Orlow
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-21 19:28 PST by Jeremy Orlow
Modified: 2010-01-22 12:35 PST (History)
4 users (show)

See Also:


Attachments
Patch (56.05 KB, patch)
2010-01-22 06:19 PST, Steve Block
no flags Details | Formatted Diff | Diff
Patch (21.49 KB, patch)
2010-01-22 11:04 PST, Jeremy Orlow
fishd: review+
fishd: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 2010-01-21 19:28:45 PST
The Chromium WebKit API needs to expose storage event related data.  Without it, there's no way for the embedder to know a storage event should be fired.
Comment 1 Steve Block 2010-01-22 06:19:57 PST
Created attachment 47196 [details]
Patch
Comment 2 Steve Block 2010-01-22 06:21:30 PST
Comment on attachment 47196 [details]
Patch

Uploaded patch to wrong bug
Comment 3 Jeremy Orlow 2010-01-22 11:04:04 PST
Created attachment 47213 [details]
Patch
Comment 4 Darin Fisher (:fishd, Google) 2010-01-22 11:14:32 PST
Comment on attachment 47213 [details]
Patch

> Index: WebKit/chromium/public/WebStorageArea.h
...
> +    virtual void setItem(const WebString& key, const WebString& newValue, const WebURL& url, bool& quotaException) // Deprecated.
> +    {
> +        WebString oldValue;
> +        setItem(key, newValue, url, quotaException, trash);

s/trash/oldValue/ ?


> +    virtual void clear(const WebURL& url) // Deprecated.
> +    {
> +        WebString somethingCleared;
> +        clear(url, somethingCleared);

WebString -> bool


Please add a FIXME note about cleaning this up once the Chromium side is adapted.



> Index: WebKit/chromium/src/StorageAreaProxy.cpp
...
> +    if (static_cast<String>(oldValue) != value)

nit: I usually just write String(oldValue) in cases like this.  It results in
the same code.


Otherwise, r=me
Comment 5 Jeremy Orlow 2010-01-22 11:19:32 PST
(In reply to comment #4)
> (From update of attachment 47213 [details])
> > Index: WebKit/chromium/public/WebStorageArea.h
> ...
> > +    virtual void setItem(const WebString& key, const WebString& newValue, const WebURL& url, bool& quotaException) // Deprecated.
> > +    {
> > +        WebString oldValue;
> > +        setItem(key, newValue, url, quotaException, trash);
> 
> s/trash/oldValue/ ?
> 
> 
> > +    virtual void clear(const WebURL& url) // Deprecated.
> > +    {
> > +        WebString somethingCleared;
> > +        clear(url, somethingCleared);
> 
> WebString -> bool

Oops...note to self: webkit-patch takes a snapshot when you first start running it...so don't start it until after you've saved the file.  :-)

> 
> Please add a FIXME note about cleaning this up once the Chromium side is
> adapted.
> 
> 
> 
> > Index: WebKit/chromium/src/StorageAreaProxy.cpp
> ...
> > +    if (static_cast<String>(oldValue) != value)
> 
> nit: I usually just write String(oldValue) in cases like this.  It results in
> the same code.

Done.

Thanks.
Comment 6 WebKit Review Bot 2010-01-22 11:35:33 PST
Attachment 47213 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/206016
Comment 7 Darin Adler 2010-01-22 11:39:23 PST
(In reply to comment #4)
> > +    if (static_cast<String>(oldValue) != value)
> 
> nit: I usually just write String(oldValue) in cases like this.  It results in the same code.

I recommend a local variable instead. Because the String(x) syntax is too powerful. It can do anything a C-style cast can do and more. Assignment to a local variable only works when the conversion can be done without any typecasting.
Comment 8 Jeremy Orlow 2010-01-22 12:17:55 PST
(In reply to comment #7)
> (In reply to comment #4)
> > > +    if (static_cast<String>(oldValue) != value)
> > 
> > nit: I usually just write String(oldValue) in cases like this.  It results in the same code.
> 
> I recommend a local variable instead. Because the String(x) syntax is too
> powerful. It can do anything a C-style cast can do and more. Assignment to a
> local variable only works when the conversion can be done without any
> typecasting.

Will do.
Comment 9 Jeremy Orlow 2010-01-22 12:35:44 PST
Committed r53710: <http://trac.webkit.org/changeset/53710>