Bug 88396 - Quota API: Update IDL to make it match the latest WD
Summary: Quota API: Update IDL to make it match the latest WD
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kinuko Yasuda
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-06 00:06 PDT by Kinuko Yasuda
Modified: 2013-03-13 19:51 PDT (History)
19 users (show)

See Also:


Attachments
Patch (164.71 KB, patch)
2012-06-06 05:38 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (165.12 KB, patch)
2012-06-06 07:30 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (165.30 KB, patch)
2012-06-06 08:24 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (174.41 KB, patch)
2012-06-06 09:00 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (175.21 KB, patch)
2012-06-06 09:45 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (175.22 KB, patch)
2012-06-06 10:18 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (92.17 KB, patch)
2012-07-04 05:01 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-05 (609.01 KB, application/zip)
2012-07-04 05:40 PDT, WebKit Review Bot
no flags Details
Patch (95.09 KB, patch)
2012-07-04 11:13 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (107.19 KB, patch)
2013-03-11 09:11 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (128.86 KB, patch)
2013-03-12 00:37 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (127.57 KB, patch)
2013-03-12 01:34 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (128.77 KB, patch)
2013-03-12 02:15 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (127.35 KB, patch)
2013-03-12 03:40 PDT, Kinuko Yasuda
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kinuko Yasuda 2012-06-06 00:06:11 PDT
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)
Comment 1 Kinuko Yasuda 2012-06-06 00:08:08 PDT
- 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
Comment 2 Kinuko Yasuda 2012-06-06 05:38:50 PDT
Created attachment 146002 [details]
Patch
Comment 3 Build Bot 2012-06-06 06:06:39 PDT
Comment on attachment 146002 [details]
Patch

Attachment 146002 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12899834
Comment 4 Kinuko Yasuda 2012-06-06 07:30:43 PDT
Created attachment 146021 [details]
Patch
Comment 5 Build Bot 2012-06-06 08:04:08 PDT
Comment on attachment 146021 [details]
Patch

Attachment 146021 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12896891
Comment 6 Kinuko Yasuda 2012-06-06 08:24:23 PDT
Created attachment 146038 [details]
Patch
Comment 7 Build Bot 2012-06-06 08:52:33 PDT
Comment on attachment 146038 [details]
Patch

Attachment 146038 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12902838
Comment 8 Kinuko Yasuda 2012-06-06 09:00:25 PDT
Created attachment 146043 [details]
Patch
Comment 9 Build Bot 2012-06-06 09:34:28 PDT
Comment on attachment 146043 [details]
Patch

Attachment 146043 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12897839
Comment 10 Kinuko Yasuda 2012-06-06 09:45:07 PDT
Created attachment 146047 [details]
Patch
Comment 11 Adam Barth 2012-06-06 09:49:06 PDT
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
Comment 12 Kinuko Yasuda 2012-06-06 10:18:06 PDT
Created attachment 146056 [details]
Patch
Comment 13 Kinuko Yasuda 2012-06-06 10:19:17 PDT
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.
Comment 14 Kinuko Yasuda 2012-06-07 03:35:24 PDT
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
Comment 15 Kinuko Yasuda 2012-06-07 03:55:22 PDT
Comment on attachment 146056 [details]
Patch

Will upload a new patch after the patch for bug 88512 lands.
Comment 16 Kinuko Yasuda 2012-07-04 05:00:07 PDT
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/
Comment 17 Kinuko Yasuda 2012-07-04 05:01:42 PDT
Created attachment 150774 [details]
Patch
Comment 18 WebKit Review Bot 2012-07-04 05:39:59 PDT
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
Comment 19 WebKit Review Bot 2012-07-04 05:40:03 PDT
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
Comment 20 Kinuko Yasuda 2012-07-04 11:13:57 PDT
Created attachment 150825 [details]
Patch
Comment 21 Brady Eidson 2012-10-29 07:21:20 PDT
This patch is pretty stale at this point.  I'm not sure if it'd be worthwhile to review in its current form.
Comment 22 Adam Barth 2012-10-29 12:21:55 PDT
Comment on attachment 150825 [details]
Patch

Ok, sounds like the previous comment is an r-.
Comment 23 Brady Eidson 2012-10-29 14:30:53 PDT
(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.  :)
Comment 24 Kinuko Yasuda 2012-10-30 03:44:11 PDT
Yup, the patch was pretty old... I'm planning to upload a new patch.
Comment 25 Kinuko Yasuda 2013-03-11 09:11:31 PDT
Created attachment 192483 [details]
Patch
Comment 26 Kinuko Yasuda 2013-03-11 09:15:47 PDT
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!
Comment 27 kov's GTK+ EWS bot 2013-03-11 09:23:22 PDT
Comment on attachment 192483 [details]
Patch

Attachment 192483 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17113619
Comment 28 WebKit Review Bot 2013-03-11 10:17:54 PDT
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
Comment 29 WebKit Review Bot 2013-03-11 11:15:56 PDT
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
Comment 30 Build Bot 2013-03-11 20:16:25 PDT
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
Comment 31 Kinuko Yasuda 2013-03-12 00:37:20 PDT
Created attachment 192656 [details]
Patch
Comment 32 Kinuko Yasuda 2013-03-12 01:34:48 PDT
Created attachment 192668 [details]
Patch
Comment 33 kov's GTK+ EWS bot 2013-03-12 01:49:21 PDT
Comment on attachment 192668 [details]
Patch

Attachment 192668 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17137139
Comment 34 Kinuko Yasuda 2013-03-12 02:15:46 PDT
Created attachment 192676 [details]
Patch
Comment 35 Mike West 2013-03-12 03:02:41 PDT
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.
Comment 36 Mike West 2013-03-12 03:04:31 PDT
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. :)
Comment 37 Kinuko Yasuda 2013-03-12 03:40:09 PDT
Created attachment 192695 [details]
Patch
Comment 38 Mike West 2013-03-12 03:43:25 PDT
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. :)
Comment 39 Kinuko Yasuda 2013-03-12 03:43:37 PDT
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.
Comment 40 Kinuko Yasuda 2013-03-12 03:45:05 PDT
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.
Comment 41 Adam Barth 2013-03-13 13:29:10 PDT
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.
Comment 42 Kinuko Yasuda 2013-03-13 19:51:27 PDT
Committed r145782: <http://trac.webkit.org/changeset/145782>