Bug 33985 – The Chromium WebKit API needs to expose storage event related data

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Jeremy Orlow]

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.

[Steve Block]

Created attachment 47196 [details]
Patch

[Steve Block]

Comment on attachment 47196 [details]
Patch

Uploaded patch to wrong bug

[Jeremy Orlow]

Created attachment 47213 [details]
Patch

[Darin Fisher (:fishd, Google)]

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

[Jeremy Orlow]

(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.

[WebKit Review Bot]

Attachment 47213 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/206016

[Darin Adler]

(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.

[Jeremy Orlow]

(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.

[Jeremy Orlow]

Committed r53710: <http://trac.webkit.org/changeset/53710>