[webkit-reviews] review denied: [Bug 29257] Chromium needs to be able to inject its own storage event handler into StorageArea : [Attachment 39575] Patch v1

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 15 16:31:31 PDT 2009


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Jeremy Orlow
<jorlow at chromium.org>'s request for review:
Bug 29257: Chromium needs to be able to inject its own storage event handler
into StorageArea
https://bugs.webkit.org/show_bug.cgi?id=29257

Attachment 39575: Patch v1
https://bugs.webkit.org/attachment.cgi?id=39575&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
> Index: WebCore/page/DOMWindow.cpp
...
> +    RefPtr<StorageArea> storageArea =
page->group().localStorage()->storageArea(document->securityOrigin(), new
StorageEventDispatcherImpl); 

I think it is generally better to use the Foo::create() pattern that returns
a PassRefPtr instead of passing a raw pointer.	That way the ownership exchange

is more clearly documented and enforced by the code.


> Index: WebCore/storage/StorageEventDispatcher.h

> +#include "PlatformString.h"
> +#include "StorageArea.h"
> +
> +#include <wtf/RefCounted.h>

nit: no new line after StorageArea.h


Otherwise, LG


More information about the webkit-reviews mailing list