Bug 137839

Summary: Don't create cached functions that access lexicalGlobalObject()
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: WebCore JavaScriptAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 136724, 136901, 137907    
Attachments:
Description Flags
Patch ggaren: review+

Michael Saboff
Reported 2014-10-17 15:05:19 PDT
The current implementation of lexicalGlobalObject() uses the JSScope stored in the current call frame. For native functions, the JSScope is copied from the caller's frame. These two behaviors together mean that a native function that calls lexicalGlobalObject() will not get their true lexicalGlobalObject, but their caller's. As part of eliminating the JSScope slot from the call frame, we need to clean up code that depends on this behavior. Therefore native functions that access their lexicalGlobalObject need to always be created for each instance of their prototype object, so that their instance has their lexicalGlobalObject.
Attachments
Patch (22.16 KB, patch)
2014-10-17 15:40 PDT, Michael Saboff 		ggaren: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Michael Saboff
Comment 1 2014-10-17 15:40:44 PDT
Created attachment 240045 [details] Patch
Geoffrey Garen
Comment 2 2014-10-17 15:45:56 PDT
Comment on attachment 240045 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240045&action=review r=me > LayoutTests/ChangeLog:17 > + a parents and child windows instances are the same. Now they are always different. "parent's" "window's"
Michael Saboff
Comment 3 2014-10-17 15:58:30 PDT
Committed r174847: <http://trac.webkit.org/changeset/174847>
Darin Adler
Comment 4 2014-10-17 23:20:49 PDT
Comment on attachment 240045 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240045&action=review > Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:65 > + if (propertyName == Identifier(exec, "open")) { I believe this is less efficient than: if (propertyName == "open") { > Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:76 > + const HashTableValue* entry = JSHTMLDocument::info()->staticPropHashTable->entry(propertyName); > + if (entry) { Would be nice to put this definition inside the if.
Michael Saboff
Comment 5 2014-10-20 10:55:39 PDT
(In reply to comment #4) > Comment on attachment 240045 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240045&action=review > > > Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:65 > > + if (propertyName == Identifier(exec, "open")) { > > I believe this is less efficient than: > > if (propertyName == "open") { > > > Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:76 > > + const HashTableValue* entry = JSHTMLDocument::info()->staticPropHashTable->entry(propertyName); > > + if (entry) { > > Would be nice to put this definition inside the if. Tracked in <https://bugs.webkit.org/show_bug.cgi?id=137885>
Note You need to log in before you can comment on or make changes to this bug.