Bug 94802

Summary: successCallback of ResolveLocalFileSystemURL should not be optional.
Product: WebKit Reporter: Taiju Tsuiki <tzik>
Component: WebKit Misc.Assignee: Taiju Tsuiki <tzik>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, ericu, kinuko, ojan, tkent, webkit.review.bot
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from gce-cr-linux-04
none
Patch
none
Archive of layout-test-results from gce-cr-linux-07
none
Patch none

Description Taiju Tsuiki 2012-08-23 05:52:23 PDT
Spec says, successCallback parameter of ResolveLocalFileSystemURL is not optional.
http://www.w3.org/TR/file-system-api/#widl-LocalFileSystem-resolveLocalFileSystemURL-void-DOMString-url-EntryCallback-successCallback-ErrorCallback-errorCallback
Comment 1 Taiju Tsuiki 2012-08-23 05:53:37 PDT
Created attachment 160140 [details]
Patch
Comment 2 Kinuko Yasuda 2012-08-23 06:10:18 PDT
Comment on attachment 160140 [details]
Patch

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

> Source/WebCore/ChangeLog:5
> +

Please add a link to the spec and description why we need to remove optional flag.
Comment 3 WebKit Review Bot 2012-08-23 06:34:52 PDT
Comment on attachment 160140 [details]
Patch

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

New failing tests:
fast/filesystem/filesystem-no-callback-null-ptr-crash.html
Comment 4 WebKit Review Bot 2012-08-23 06:34:54 PDT
Created attachment 160148 [details]
Archive of layout-test-results from gce-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 5 Taiju Tsuiki 2012-08-23 06:42:12 PDT
Created attachment 160149 [details]
Patch
Comment 6 Taiju Tsuiki 2012-08-23 06:43:35 PDT
Comment on attachment 160140 [details]
Patch

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

>> Source/WebCore/ChangeLog:5
>> +
> 
> Please add a link to the spec and description why we need to remove optional flag.

Done
Comment 7 WebKit Review Bot 2012-08-23 07:26:05 PDT
Comment on attachment 160149 [details]
Patch

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

New failing tests:
fast/filesystem/filesystem-no-callback-null-ptr-crash.html
Comment 8 WebKit Review Bot 2012-08-23 07:26:08 PDT
Created attachment 160156 [details]
Archive of layout-test-results from gce-cr-linux-07

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-07  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 9 Kent Tamura 2012-08-23 18:45:33 PDT
Comment on attachment 160149 [details]
Patch

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

> LayoutTests/ChangeLog:11
> +        * fast/filesystem/filesystem-no-callback-null-ptr-crash.html:

This test is for a crash with a particular scenario. We should have another conformance test for existence of the successCallback argument.

> LayoutTests/fast/filesystem/filesystem-no-callback-null-ptr-crash.html:19
> -      webkitResolveLocalFileSystemURL('');
>        try {
> -          webkitRequestFileSystem(TEMPORARY, 100);
> -      }
> +          webkitResolveLocalFileSystemURL('');
>        catch(e) {
> -          document.getElementById('log').innerHTML = "PASS";
> +          try {
> +              webkitRequestFileSystem(TEMPORARY, 100);
> +          }
> +          catch(e) {
> +              document.getElementById('log').innerHTML = "PASS";
> +          }

This looks to change the testing scenario.
Comment 10 Taiju Tsuiki 2012-08-23 23:17:45 PDT
Created attachment 160337 [details]
Patch
Comment 11 Taiju Tsuiki 2012-08-23 23:20:56 PDT
Comment on attachment 160149 [details]
Patch

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

>> LayoutTests/ChangeLog:11
>> +        * fast/filesystem/filesystem-no-callback-null-ptr-crash.html:
> 
> This test is for a crash with a particular scenario. We should have another conformance test for existence of the successCallback argument.

Done

>> LayoutTests/fast/filesystem/filesystem-no-callback-null-ptr-crash.html:19
>> +          }
> 
> This looks to change the testing scenario.

I see. Reverted.
Comment 12 Kent Tamura 2012-08-24 01:38:58 PDT
Comment on attachment 160337 [details]
Patch

Looks ok.
Comment 13 WebKit Review Bot 2012-09-09 23:22:13 PDT
Comment on attachment 160337 [details]
Patch

Clearing flags on attachment: 160337

Committed r128018: <http://trac.webkit.org/changeset/128018>
Comment 14 WebKit Review Bot 2012-09-09 23:22:16 PDT
All reviewed patches have been landed.  Closing bug.