[Webkit-unassigned] [Bug 57927] Expose unified Quota API if QUOTA build flag is enabled

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=57927


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #88442|review?                     |review-
               Flag|                            |




--- Comment #2 from David Levin <levin at chromium.org>  2011-04-06 11:06:10 PST ---
(From update of attachment 88442)
View in context: https://bugs.webkit.org/attachment.cgi?id=88442&action=review

> Source/WebCore/ChangeLog:15
> +        No new tests: tests will be added when we add the implementation.

Usually the idl is the last thing added (after the implementation) and tests are required when the idl is added.


Maybe you should do the changes without the idl and add it in with tests when the impl is there.

> Source/WebCore/page/DOMWindow.cpp:111
> +#if ENABLE(QUOTA)

No if needed here. (The header has it.) Just put the include with the others.

> Source/WebCore/storage/StorageInfo.cpp:38
> +#include "ScriptExecutionContext.h"

Not needed a fwd decl will do.

> Source/WebCore/storage/StorageInfo.h:49
> +        TEMPORARY,

Enum members should user InterCaps with an initial capital letter. (I think you're using a chromium style.)

> Source/WebCore/storage/StorageInfo.h:58
> +    ~StorageInfo();

Consider making this private and making RefCounted<StorageInfo> a friend.

> Source/WebCore/storage/StorageInfo.idl:2
> + * Copyright (C) 2008, 2009 Apple Inc. All rights reserved.

This is an odd copyright header.

> Source/WebKit/chromium/src/StorageInfoChromium.cpp:57
> +    }

What if !isDocument?

I'm bothered by the raw pointer (new...) but I understand. I wonder if there is another way to do this.

> Source/WebKit/chromium/src/StorageInfoChromium.cpp:67
> +    }

Ditto.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.