Bug 172724

Summary:

convertEnumerationToJS() should not stash ASCIILiteral strings in NeverDestroyed arrays.

Product:

WebKit

Reporter:

Mark Lam <mark.lam>

Component:

JavaScriptCore

Assignee:

Mark Lam <mark.lam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, cdumez, ddkilzer, joepeck, rniwa, sam, simon.fraser, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
proposed patch.	mark.lam: review-
proposed patch w/ bindings tests rebaselined.	cdumez: review+, buildbot: commit-queue-
Archive of layout-test-results from ews112 for mac-elcapitan	none

Mark Lam

Reported 2017-05-30 10:55:49 PDT

Use MAKE_STATIC_STRING_IMPL instead. <rdar://problem/31193201>

Attachments
proposed patch. (2.79 KB, patch) 2017-05-30 11:45 PDT, Mark Lam	mark.lam: review-	Details Formatted Diff Diff
proposed patch w/ bindings tests rebaselined. (11.80 KB, patch) 2017-05-30 11:51 PDT, Mark Lam	cdumez: review+ buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews112 for mac-elcapitan (1.58 MB, application/zip) 2017-05-30 13:21 PDT, Build Bot	no flags	Details
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Mark Lam

Comment 1 2017-05-30 11:45:00 PDT

Created attachment 311522 [details] proposed patch.

Mark Lam

Comment 2 2017-05-30 11:47:56 PDT

Comment on attachment 311522 [details] proposed patch. Need to rebase bindings tests. Will update patch shortly.

Chris Dumez

Comment 3 2017-05-30 11:48:54 PDT

Comment on attachment 311522 [details] proposed patch. You will likely need to rebaseline the bindings tests.

Mark Lam

Comment 4 2017-05-30 11:51:40 PDT

Created attachment 311525 [details] proposed patch w/ bindings tests rebaselined.

Chris Dumez

Comment 5 2017-05-30 11:53:06 PDT

Comment on attachment 311525 [details] proposed patch w/ bindings tests rebaselined. r=me

Joseph Pecoraro

Comment 6 2017-05-30 11:54:20 PDT

View in context: https://bugs.webkit.org/attachment.cgi?id=311525&action=review r=me as well > Source/WebCore/ChangeLog:17 > + whose root cause is not known yet. This patch also adds a release assert to Nit: One space after a period!

Mark Lam

Comment 7 2017-05-30 11:55:47 PDT

(In reply to Joseph Pecoraro from comment #6) > > Source/WebCore/ChangeLog:17 > > + whose root cause is not known yet. This patch also adds a release assert to > > Nit: One space after a period! Not in a ChangeLog. In a ChangeLog, I'm permitted to use the more readable style of 2 spaces between sentences. =)

Build Bot

Comment 8 2017-05-30 13:21:24 PDT

Comment on attachment 311525 [details] proposed patch w/ bindings tests rebaselined. Attachment 311525 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3843663 New failing tests: imported/w3c/web-platform-tests/innerText/getter.html

Build Bot

Comment 9 2017-05-30 13:21:26 PDT

Created attachment 311535 [details] Archive of layout-test-results from ews112 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Mark Lam

Comment 10 2017-05-30 13:29:20 PDT

(In reply to Build Bot from comment #8) > Comment on attachment 311525 [details] > proposed patch w/ bindings tests rebaselined. > > Attachment 311525 [details] did not pass mac-debug-ews (mac): > Output: http://webkit-queues.webkit.org/results/3843663 > > New failing tests: > imported/w3c/web-platform-tests/innerText/getter.html This is not due to my patch. See https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK2%20(Tests)/r217560%20(1262)/results.html. This issue is already failing in trunk without my patch.

Mark Lam

Comment 11 2017-05-30 13:36:01 PDT

Thanks for the review. Landed in r217568: <http://trac.webkit.org/r217568>.

Note You need to log in before you can comment on or make changes to this bug.