[webkit-reviews] review denied: [Bug 46648] Support different local and session storage quota for different security origins. : [Attachment 193971] patch without changing chromium public API.

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Lyon Chen <liachen at blackberry.com>'s
request for review:
Bug 46648: Support different local and session storage quota for different
security origins.
https://bugs.webkit.org/show_bug.cgi?id=46648

Attachment 193971: patch without changing chromium public API.
https://bugs.webkit.org/attachment.cgi?id=193971&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=193971&action=review


> Source/WebCore/page/ChromeClient.h:192
> +	   enum StorageType {

bad indent

> Source/WebCore/page/ChromeClient.h:193
> +	       StorageLocal = 0,

No need for = 0 here.  The compiler will handle that for you.

> Source/WebCore/page/ChromeClient.h:198
> +	   virtual bool shouldAllowStorageRequest(StorageType, const
SecurityOrigin&, unsigned quota) = 0;

const SecurityOrigin& -> SecurityOrigin*

We pass SecurityOrigin objects by pointer, not by reference.

Why is this function on the ChromeClient?  It has nothing to do with the
browser's chrome.

> Source/WebCore/page/DOMWindow.cpp:771
> +    if (!storageArea.get()) {

There's no need to call get() here.

> Source/WebCore/page/DOMWindow.cpp:773
> +	   if (!page->chrome() || !page->chrome()->client())
> +	       return 0;

How can these ever be 0?