Bug 160290 - Fetch Response built-ins should use @makeThisTypeError
Summary: Fetch Response built-ins should use @makeThisTypeError
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks: 151937
  Show dependency treegraph
 
Reported: 2016-07-28 03:23 PDT by youenn fablet
Modified: 2016-07-31 23:02 PDT (History)
3 users (show)

See Also:


Attachments
Patch (6.86 KB, patch)
2016-07-28 03:26 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (795.31 KB, application/zip)
2016-07-28 04:14 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (896.94 KB, application/zip)
2016-07-28 04:17 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (647.66 KB, application/zip)
2016-07-28 04:23 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-yosemite (1.43 MB, application/zip)
2016-07-28 04:25 PDT, Build Bot
no flags Details
Fixing test expectation (6.92 KB, patch)
2016-07-28 04:32 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (7.58 KB, patch)
2016-07-29 06:37 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.