Bug 134126

Summary: Remove static tables for bindings that use eager reification
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, buildbot, cdumez, cgarcia, commit-queue, eric.carlson, glenn, jer.noble, jsbell, philipj, rniwa, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
none
Patch oliver: review+

Description Sam Weinig 2014-06-20 14:41:01 PDT
Remove static tables for bindings that use eager reification
Comment 1 Sam Weinig 2014-06-20 14:43:06 PDT
Created attachment 233459 [details]
Patch
Comment 2 Build Bot 2014-06-20 16:16:01 PDT
Comment on attachment 233459 [details]
Patch

Attachment 233459 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5140483131047936

New failing tests:
js/dom/webidl-type-mapping.html
Comment 3 Build Bot 2014-06-20 16:16:06 PDT
Created attachment 233478 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 4 Build Bot 2014-06-20 16:34:01 PDT
Comment on attachment 233459 [details]
Patch

Attachment 233459 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6407120526245888

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html
js/dom/webidl-type-mapping.html
Comment 5 Build Bot 2014-06-20 16:34:07 PDT
Created attachment 233485 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 6 Sam Weinig 2014-06-20 16:56:12 PDT
Comment on attachment 233459 [details]
Patch

This is currently breaking js/dom/webidl-type-mapping.html.  Taking a look.
Comment 7 Sam Weinig 2014-06-20 18:03:09 PDT
Created attachment 233493 [details]
Patch
Comment 8 WebKit Commit Bot 2014-06-20 18:05:46 PDT
Attachment 233493 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/Structure.h:284:  is__proto__ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 39 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Sam Weinig 2014-06-20 18:10:16 PDT
(In reply to comment #6)
> (From update of attachment 233459 [details])
> This is currently breaking js/dom/webidl-type-mapping.html.  Taking a look.

The issue, for those playing along at home, was that when eagerly reifying properties we were not setting the m_hasReadOnlyOrGetterSetterPropertiesExcludingProto bit on the Structure.  

The reason was that when we do this eager reification, we call JSObject::putDirectCustomAccessor(...) which in turn calls Structure::setHasCustomGetterSetterProperties() (this mirrors the path taken when add a normal getter/setter which calls JSObject::putDirectAccessor(...) which calls Structure::setHasGetterSetterProperties()) but setHasCustomGetterSetterProperties wasn't setting the bit as it should have been.

This was not a problem in the past because all use of eager reification had left the static HashTables hanging off their ClassInfo, which initialized the m_hasReadOnlyOrGetterSetterPropertiesExcludingProto bit at Structure creation time.
Comment 10 Alexey Proskuryakov 2014-06-20 20:50:02 PDT
See also: bug 133687.
Comment 11 Sam Weinig 2014-06-21 16:42:52 PDT
Fixed in http://trac.webkit.org/changeset/170256.