Bug 204629 - Remove own toString from NativeError prototype
Summary: Remove own toString from NativeError prototype
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-11-26 14:42 PST by Gus Caplan
Modified: 2020-01-20 19:49 PST (History)
12 users (show)

See Also:


Attachments
Patch (6.88 KB, patch)
2019-11-26 14:43 PST, Gus Caplan
no flags Details | Formatted Diff | Diff
Patch (6.86 KB, patch)
2019-11-26 15:04 PST, Gus Caplan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews210 for win-future (4.36 MB, application/zip)
2019-11-26 16:25 PST, EWS Watchlist
no flags Details
Patch (6.67 KB, patch)
2019-11-26 22:05 PST, Gus Caplan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews210 for win-future (14.94 MB, application/zip)
2019-11-27 02:18 PST, EWS Watchlist
no flags Details
Patch (6.89 KB, patch)
2019-11-27 08:36 PST, Gus Caplan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews210 for win-future (14.80 MB, application/zip)
2019-11-27 10:19 PST, EWS Watchlist
no flags Details
Patch (9.43 KB, patch)
2019-11-28 19:28 PST, Gus Caplan
no flags Details | Formatted Diff | Diff
Patch (129.31 KB, patch)
2019-11-29 14:04 PST, Gus Caplan
no flags Details | Formatted Diff | Diff
Patch (129.21 KB, patch)
2020-01-19 15:45 PST, Gus Caplan
no flags Details | Formatted Diff | Diff
patch (130.26 KB, patch)
2020-01-20 14:34 PST, Gus Caplan
no flags Details | Formatted Diff | Diff
Patch (130.26 KB, patch)
2020-01-20 14:40 PST, Gus Caplan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gus Caplan 2019-11-26 14:42:39 PST
Remove own toString from NativeError prototype
Comment 1 Gus Caplan 2019-11-26 14:43:59 PST
Created attachment 384381 [details]
Patch
Comment 2 Gus Caplan 2019-11-26 15:04:50 PST
Created attachment 384383 [details]
Patch
Comment 3 Gus Caplan 2019-11-26 15:33:25 PST
It seems with the current patch, Error.prototype.toString isn't being created anymore. I admittedly have no idea how any of this works, so pointers would be appreciated.
Comment 4 EWS Watchlist 2019-11-26 16:25:32 PST Comment hidden (obsolete)
Comment 5 EWS Watchlist 2019-11-26 16:25:34 PST Comment hidden (obsolete)
Comment 6 Gus Caplan 2019-11-26 22:05:51 PST
Created attachment 384397 [details]
Patch
Comment 7 EWS Watchlist 2019-11-27 02:18:49 PST Comment hidden (obsolete)
Comment 8 EWS Watchlist 2019-11-27 02:18:52 PST Comment hidden (obsolete)
Comment 9 Gus Caplan 2019-11-27 08:36:33 PST
Created attachment 384415 [details]
Patch
Comment 10 EWS Watchlist 2019-11-27 10:19:43 PST Comment hidden (obsolete)
Comment 11 EWS Watchlist 2019-11-27 10:19:45 PST Comment hidden (obsolete)
Comment 12 Gus Caplan 2019-11-28 19:28:32 PST
Created attachment 384476 [details]
Patch
Comment 13 Gus Caplan 2019-11-29 14:04:46 PST
Created attachment 384526 [details]
Patch
Comment 14 Ross Kirsling 2019-11-29 14:44:54 PST
Comment on attachment 384526 [details]
Patch

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

r=me, though you'll need to regenerate the ChangeLogs.

> JSTests/stress/nativeerror-prototype-tostring.js:7
> +  if (NativeError.prototype.hasOwnProperty('toString')) {
> +    throw new Error(`${NativeError.name}.prototype should not have own property 'toString'`);
> +  }

Nit: WK style drops braces for one-line conditionals (https://webkit.org/code-style-guidelines/#braces-one-line).
I guess the checker ignores JS stress tests but we ought to align anyway.
Comment 15 Gus Caplan 2020-01-19 15:45:26 PST
Created attachment 388198 [details]
Patch
Comment 16 Darin Adler 2020-01-20 10:26:18 PST
Comment on attachment 388198 [details]
Patch

Code change looks pretty good. Need to fix the failing tests on EWS before we get this reviewed.
Comment 17 Ross Kirsling 2020-01-20 14:21:04 PST
Comment on attachment 388198 [details]
Patch

The EWS failure here is just due to auto-trimmed whitespace; just need to undo that and get the regenerated changelogs into the patch. :)
Comment 18 Gus Caplan 2020-01-20 14:34:29 PST
Created attachment 388258 [details]
patch
Comment 19 Gus Caplan 2020-01-20 14:40:40 PST
Created attachment 388259 [details]
Patch
Comment 20 WebKit Commit Bot 2020-01-20 19:47:43 PST
The commit-queue encountered the following flaky tests while processing attachment 388259 [details]:

editing/spelling/spellcheck-attribute.html bug 206178 (authors: g.czajkowski@samsung.com, mark.lam@apple.com, and rniwa@webkit.org)
The commit-queue is continuing to process your patch.
Comment 21 WebKit Commit Bot 2020-01-20 19:48:24 PST
Comment on attachment 388259 [details]
Patch

Clearing flags on attachment: 388259

Committed r254842: <https://trac.webkit.org/changeset/254842>
Comment 22 WebKit Commit Bot 2020-01-20 19:48:26 PST
All reviewed patches have been landed.  Closing bug.
Comment 23 Radar WebKit Bug Importer 2020-01-20 19:49:20 PST
<rdar://problem/58747998>