Bug 88396 – Quota API: Update IDL to make it match the latest WD

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

[Kinuko Yasuda]

StorageInfo interface for Quota API should be updated to reflect the latest draft

Draft: http://dvcs.w3.org/hg/quota/raw-file/tip/Overview.html

It's also nice if we could migrate it to a component (e.g. moving them under WebCore/Modules/quota)

[Kinuko Yasuda]

- Now the quota access interface is attached to window.navigator rather than window

- Now it has no enum for TEMPORARY/PERSISTENT.  Instead now we have:
  navigator.persistentStorage and
  navigator.temporaryStorage

- Interface name changes:
  QuotaStorageEnvironment -> StorageQuotaEnvironment
  StorageInfo -> StorageQuota
  StorageInfoQuotaCallback -> StorageQuotaCallback
  StorageInfoUsageCallback -> StorageUsageCallback
  StorageInfoErrorCallback -> StorageErrorCallback

[Kinuko Yasuda]

Created attachment 146002 [details]
Patch

[Build Bot]

Comment on attachment 146002 [details]
Patch

Attachment 146002 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12899834

[Kinuko Yasuda]

Created attachment 146021 [details]
Patch

[Build Bot]

Comment on attachment 146021 [details]
Patch

Attachment 146021 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12896891

[Kinuko Yasuda]

Created attachment 146038 [details]
Patch

[Build Bot]

Comment on attachment 146038 [details]
Patch

Attachment 146038 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12902838

[Kinuko Yasuda]

Created attachment 146043 [details]
Patch

[Build Bot]

Comment on attachment 146043 [details]
Patch

Attachment 146043 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12897839

[Kinuko Yasuda]

Created attachment 146047 [details]
Patch

[Adam Barth]

Comment on attachment 146047 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=146047&action=review

> Source/WebCore/Modules/quota/NavigatorStorageQuota.h:56
> +    NavigatorStorageQuota(Frame*);

explicit

[Kinuko Yasuda]

Created attachment 146056 [details]
Patch

[Kinuko Yasuda]

Comment on attachment 146047 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=146047&action=review

>> Source/WebCore/Modules/quota/NavigatorStorageQuota.h:56
>> +    NavigatorStorageQuota(Frame*);
> 
> explicit

Done.

[Kinuko Yasuda]

Since this patch contains two changes (file moving + renaming) I separated out the file moving part (WebCore/storage -> WebCore/Modules/quota) into a new bug:
bug 88512

[Kinuko Yasuda]

Comment on attachment 146056 [details]
Patch

Will upload a new patch after the patch for bug 88512 lands.

[Kinuko Yasuda]

Since now the FPWD for the Quota Management API has been published we should update the WebKit implementation to match with the draft asap:
http://www.w3.org/TR/2012/WD-quota-api-20120703/

[Kinuko Yasuda]

Created attachment 150774 [details]
Patch

[WebKit Review Bot]

Comment on attachment 150774 [details]
Patch

Attachment 150774 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13131786

New failing tests:
fast/dom/everything-to-string.html
http/tests/security/isolatedWorld/all-window-properties.html
fast/dom/navigator-detached-no-crash.html
fast/js/global-constructors.html
http/tests/security/isolatedWorld/all-window-prototypes.html

[WebKit Review Bot]

Created attachment 150777 [details]
Archive of layout-test-results from gce-cr-linux-05

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-05  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Kinuko Yasuda]

Created attachment 150825 [details]
Patch

[Brady Eidson]

This patch is pretty stale at this point.  I'm not sure if it'd be worthwhile to review in its current form.

[Adam Barth]

Comment on attachment 150825 [details]
Patch

Ok, sounds like the previous comment is an r-.

[Brady Eidson]

(In reply to comment #22)
> (From update of attachment 150825 [details])
> Ok, sounds like the previous comment is an r-.

That was the strong implication, yes, but I didn't have the heart to formalize it.  :)

[Kinuko Yasuda]

Yup, the patch was pretty old... I'm planning to upload a new patch.

[Kinuko Yasuda]

Created attachment 192483 [details]
Patch

[Kinuko Yasuda]

I finally had time to update this patch... sorry for the slow update. Could Adam, Brady or someone take a look at the new patch?  Thanks so much!

[kov's GTK+ EWS bot]

Comment on attachment 192483 [details]
Patch

Attachment 192483 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17113619

[WebKit Review Bot]

Comment on attachment 192483 [details]
Patch

Attachment 192483 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17161224

New failing tests:
fast/dom/navigator-detached-no-crash.html

[WebKit Review Bot]

Comment on attachment 192483 [details]
Patch

Attachment 192483 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17011621

New failing tests:
fast/dom/navigator-detached-no-crash.html

[Build Bot]

Comment on attachment 192483 [details]
Patch

Attachment 192483 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17013958

New failing tests:
compositing/overflow/composited-scrolling-paint-phases.html
storage/storagequota-query-usage.html
storage/storagequota-request-quota.html
editing/selection/selection-modify-crash.html

[Kinuko Yasuda]

Created attachment 192656 [details]
Patch

[Kinuko Yasuda]

Created attachment 192668 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 192668 [details]
Patch

Attachment 192668 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17137139

[Kinuko Yasuda]

Created attachment 192676 [details]
Patch

[Mike West]

Comment on attachment 192676 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192676&action=review

> Source/WebCore/Modules/quota/DOMWindowQuota.cpp:77
> +        scriptExecutionContext->addConsoleMessage(JSMessageSource, WarningMessageLevel, "window.webkitStorageInfo is deprecated. Use navigator.webkitTemporaryStorage or navigator.webkitPersistentStorage instead.");

Drive-by: You already have a DOMWindow here; I'm fairly certain you can just grab the DOMWindow::document() directly and call Document::addMessage() without passing the ScriptExecutionContext around.

[Mike West]

Comment on attachment 192676 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192676&action=review

>> Source/WebCore/Modules/quota/DOMWindowQuota.cpp:77
>> +        scriptExecutionContext->addConsoleMessage(JSMessageSource, WarningMessageLevel, "window.webkitStorageInfo is deprecated. Use navigator.webkitTemporaryStorage or navigator.webkitPersistentStorage instead.");
> 
> Drive-by: You already have a DOMWindow here; I'm fairly certain you can just grab the DOMWindow::document() directly and call Document::addMessage() without passing the ScriptExecutionContext around.

Sorry: I meant that you already have a Frame. Same point applies. :)

[Kinuko Yasuda]

Created attachment 192695 [details]
Patch

[Mike West]

Comment on attachment 192695 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192695&action=review

> Source/WebCore/Modules/quota/DOMWindowQuota.cpp:39
> +#include "ScriptExecutionContext.h"

Nit: I think you can drop this header as well. :)

[Kinuko Yasuda]

Comment on attachment 192676 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192676&action=review

>>> Source/WebCore/Modules/quota/DOMWindowQuota.cpp:77
>>> +        scriptExecutionContext->addConsoleMessage(JSMessageSource, WarningMessageLevel, "window.webkitStorageInfo is deprecated. Use navigator.webkitTemporaryStorage or navigator.webkitPersistentStorage instead.");
>> 
>> Drive-by: You already have a DOMWindow here; I'm fairly certain you can just grab the DOMWindow::document() directly and call Document::addMessage() without passing the ScriptExecutionContext around.
> 
> Sorry: I meant that you already have a Frame. Same point applies. :)

You're right! I reverted the changes in DOMWindowQuota.{h,idl} and updated the .cpp code to use frame()->document(). Thanks for catching this.

[Kinuko Yasuda]

Comment on attachment 192695 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192695&action=review

>> Source/WebCore/Modules/quota/DOMWindowQuota.cpp:39
>> +#include "ScriptExecutionContext.h"
> 
> Nit: I think you can drop this header as well. :)

Thx, I'll fix this in the next patch.

[Adam Barth]

Comment on attachment 192695 [details]
Patch

This patch looked scary from the outside, but after digging into it, it looks much more straightforward.  Sorry about the delay in reviewing.

[Kinuko Yasuda]

Committed r145782: <http://trac.webkit.org/changeset/145782>