[Webkit-unassigned] [Bug 57927] Add mock implementation and plumbing code for unified Quota API

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Kinuko Yasuda <kinuko at chromium.org>  2011-04-06 21:53:42 PST ---
(From update of attachment 88442)
Thanks for reviewing,

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.

Discarded the idl changes.

>> Source/WebCore/page/DOMWindow.cpp:111
>> +#if ENABLE(QUOTA)
> 
> No if needed here. (The header has it.) Just put the include with the others.

Discarded this change for now.

>> Source/WebCore/storage/StorageInfo.cpp:38
>> +#include "ScriptExecutionContext.h"
> 
> Not needed a fwd decl will do.

Done.

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

They're named after the IDL's constant names to enable enum checks.  (I removed the IDL changes from the patch so I removed this enum for now too.)

>> Source/WebCore/storage/StorageInfo.h:58
>> +    ~StorageInfo();
> 
> Consider making this private and making RefCounted<StorageInfo> a friend.

Done.

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

Will fix. (The IDL changes have been discarded from this change)

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

Added error handling code for !document cases.  As for the raw pointer I made a wrapper create method to make it less naked...

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

Did the same.

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