Bug 158431

Summary:

Remove hasStaticPropertyTable, or make it constexpr

Product:

WebKit

Reporter:

Gavin Barraclough <barraclough>

Component:

Bindings

Assignee:

Gavin Barraclough <barraclough>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, ggaren, rniwa

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Remove hasStaticPropertyTable (part 1: DOM bindings)	cdumez: review+
Remove hasStaticPropertyTable (part 2: JSPluginElement)	cdumez: review+
Remove hasStaticPropertyTable (part 3: JSLocation::putDelegate)	buildbot: commit-queue-
Remove hasStaticPropertyTable (part 4: JSHTMLDocument & JSStorage)	cdumez: review+
Archive of layout-test-results from ews112 for mac-yosemite	none
Remove hasStaticPropertyTable (part 3: JSLocation::putDelegate)	ggaren: review+
Remove hasStaticPropertyTable (part 5: done!)	cdumez: review+

Gavin Barraclough

Reported 2016-06-06 12:08:05 PDT

I think this is mostly used in asserts, which from a quick look are mostly wrong wrong. If we do need be checking anything, we can probably use the HasStaticPropertyTable flag in JSTypeInfo. If this is not removed, should be made constexpr.

Attachments
Remove hasStaticPropertyTable (part 1: DOM bindings) (2.61 KB, patch) 2016-06-11 21:42 PDT, Gavin Barraclough	cdumez: review+	Details Formatted Diff Diff
Remove hasStaticPropertyTable (part 2: JSPluginElement) (2.39 KB, patch) 2016-06-11 21:43 PDT, Gavin Barraclough	cdumez: review+	Details Formatted Diff Diff
Remove hasStaticPropertyTable (part 3: JSLocation::putDelegate) (4.64 KB, patch) 2016-06-11 21:43 PDT, Gavin Barraclough	buildbot: commit-queue-	Details Formatted Diff Diff
Remove hasStaticPropertyTable (part 4: JSHTMLDocument & JSStorage) (3.52 KB, patch) 2016-06-11 21:44 PDT, Gavin Barraclough	cdumez: review+	Details Formatted Diff Diff
Archive of layout-test-results from ews112 for mac-yosemite (1.43 MB, application/zip) 2016-06-11 22:46 PDT, Build Bot	no flags	Details
Remove hasStaticPropertyTable (part 3: JSLocation::putDelegate) (4.67 KB, patch) 2016-06-11 22:50 PDT, Gavin Barraclough	ggaren: review+	Details Formatted Diff Diff
Remove hasStaticPropertyTable (part 5: done!) (17.59 KB, patch) 2016-06-14 00:44 PDT, Gavin Barraclough	cdumez: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Gavin Barraclough

Comment 1 2016-06-11 21:42:24 PDT

Created attachment 281119 [details] Remove hasStaticPropertyTable (part 1: DOM bindings)

Gavin Barraclough

Comment 2 2016-06-11 21:43:00 PDT

Created attachment 281120 [details] Remove hasStaticPropertyTable (part 2: JSPluginElement)

Gavin Barraclough

Comment 3 2016-06-11 21:43:38 PDT

Created attachment 281121 [details] Remove hasStaticPropertyTable (part 3: JSLocation::putDelegate)

Gavin Barraclough

Comment 4 2016-06-11 21:44:07 PDT

Created attachment 281122 [details] Remove hasStaticPropertyTable (part 4: JSHTMLDocument & JSStorage)

Build Bot

Comment 5 2016-06-11 22:46:16 PDT

Comment on attachment 281121 [details] Remove hasStaticPropertyTable (part 3: JSLocation::putDelegate) Attachment 281121 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1487534 New failing tests: storage/indexeddb/transaction-complete-workers.html

Build Bot

Comment 6 2016-06-11 22:46:18 PDT

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

Gavin Barraclough

Comment 7 2016-06-11 22:50:33 PDT

Created attachment 281124 [details] Remove hasStaticPropertyTable (part 3: JSLocation::putDelegate)

Chris Dumez

Comment 8 2016-06-13 09:07:39 PDT

Comment on attachment 281119 [details] Remove hasStaticPropertyTable (part 1: DOM bindings) R=me

Chris Dumez

Comment 9 2016-06-13 09:10:34 PDT

Comment on attachment 281120 [details] Remove hasStaticPropertyTable (part 2: JSPluginElement) R=me, I confirmed this is indeed a no-op.

Chris Dumez

Comment 10 2016-06-13 09:11:49 PDT

Comment on attachment 281122 [details] Remove hasStaticPropertyTable (part 4: JSHTMLDocument & JSStorage) R=me, I added those :) Do we still need the hasStaticPropertyTable flag in the bindings?

Gavin Barraclough

Comment 11 2016-06-13 10:07:44 PDT

(In reply to comment #10) > Comment on attachment 281122 [details] > Remove hasStaticPropertyTable (part 4: JSHTMLDocument & JSStorage) > > R=me, I added those :) Do we still need the hasStaticPropertyTable flag in > the bindings? No - won't need any more - after I've landed these 4 I'll upload a 5th patch to remove from the bindings.

Geoffrey Garen

Comment 12 2016-06-13 13:52:27 PDT

Comment on attachment 281124 [details] Remove hasStaticPropertyTable (part 3: JSLocation::putDelegate) r=me

Gavin Barraclough

Comment 13 2016-06-13 23:28:14 PDT

Part 1: Committed revision 202028. Part 2: Committed revision 202029. Part 3: Committed revision 202030. Part 4: Committed revision 202031.

Gavin Barraclough

Comment 14 2016-06-13 23:35:19 PDT

Build fix in r202032.

Gavin Barraclough

Comment 15 2016-06-14 00:44:27 PDT

Created attachment 281242 [details] Remove hasStaticPropertyTable (part 5: done!)

Chris Dumez

Comment 16 2016-06-14 09:16:25 PDT

Comment on attachment 281242 [details] Remove hasStaticPropertyTable (part 5: done!) R=me

Gavin Barraclough

Comment 17 2016-06-19 17:04:12 PDT

Committed revision 202218.

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