Bug 160290

Summary: Fetch Response built-ins should use @makeThisTypeError
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 151937    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews101 for mac-yosemite
none
Archive of layout-test-results from ews105 for mac-yosemite-wk2
none
Archive of layout-test-results from ews123 for ios-simulator-wk2
none
Archive of layout-test-results from ews117 for mac-yosemite
none
Fixing test expectation
none
Patch none

Description youenn fablet 2016-07-28 03:23:39 PDT
Fetch Response built-ins should use @makeThisTypeError
Comment 1 youenn fablet 2016-07-28 03:26:44 PDT
Created attachment 284769 [details]
Patch
Comment 2 Build Bot 2016-07-28 04:14:20 PDT
Comment on attachment 284769 [details]
Patch

Attachment 284769 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1766963

New failing tests:
fetch/fetch-error-messages.html
Comment 3 Build Bot 2016-07-28 04:14:22 PDT
Created attachment 284772 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 4 Build Bot 2016-07-28 04:17:00 PDT
Comment on attachment 284769 [details]
Patch

Attachment 284769 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1766964

New failing tests:
fetch/fetch-error-messages.html
Comment 5 Build Bot 2016-07-28 04:17:03 PDT
Created attachment 284773 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 6 Build Bot 2016-07-28 04:23:15 PDT
Comment on attachment 284769 [details]
Patch

Attachment 284769 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1766965

New failing tests:
fetch/fetch-error-messages.html
Comment 7 Build Bot 2016-07-28 04:23:18 PDT
Created attachment 284774 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.5
Comment 8 Build Bot 2016-07-28 04:24:58 PDT
Comment on attachment 284769 [details]
Patch

Attachment 284769 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1766970

New failing tests:
fetch/fetch-error-messages.html
Comment 9 Build Bot 2016-07-28 04:25:01 PDT
Created attachment 284775 [details]
Archive of layout-test-results from ews117 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 10 youenn fablet 2016-07-28 04:32:03 PDT
Created attachment 284776 [details]
Fixing test expectation
Comment 11 Alex Christensen 2016-07-28 10:58:30 PDT
Comment on attachment 284776 [details]
Fixing test expectation

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

> Source/WebCore/Modules/fetch/FetchResponse.js:102
> -    if (!this instanceof @Response)
> -        throw new @TypeError("Function should be called on a Response");
> +    if (!(this instanceof @Response))
> +        return @Promise.@reject(@makeThisTypeError("Response", "arrayBuffer"));

This seems like a significant change in behavior from throwing to rejecting.  Worth noting in ChangeLog.  Is it correct?  Why didn't you make the same change with clone?
Comment 12 youenn fablet 2016-07-29 06:17:26 PDT
(In reply to comment #11)
> Comment on attachment 284776 [details]
> Fixing test expectation
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=284776&action=review
> 
> > Source/WebCore/Modules/fetch/FetchResponse.js:102
> > -    if (!this instanceof @Response)
> > -        throw new @TypeError("Function should be called on a Response");
> > +    if (!(this instanceof @Response))
> > +        return @Promise.@reject(@makeThisTypeError("Response", "arrayBuffer"));
> 
> This seems like a significant change in behavior from throwing to rejecting.
> Worth noting in ChangeLog.  Is it correct?  Why didn't you make the same
> change with clone?

Promise returning functions should reject promises, hence why updating these functions except for clone.
I'll rebase the patch and update the change log
Comment 13 youenn fablet 2016-07-29 06:37:27 PDT
Created attachment 284861 [details]
Patch
Comment 14 WebKit Commit Bot 2016-07-31 23:02:14 PDT
Comment on attachment 284861 [details]
Patch

Clearing flags on attachment: 284861

Committed r203961: <http://trac.webkit.org/changeset/203961>
Comment 15 WebKit Commit Bot 2016-07-31 23:02:18 PDT
All reviewed patches have been landed.  Closing bug.