...
<rdar://problem/27084591>
Created attachment 283228 [details] patch
Comment on attachment 283228 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=283228&action=review > Source/JavaScriptCore/runtime/JSObject.h:1569 > + if (attributes & ReadOnly) > + structure->setContainsReadOnlyProperties(); I put this here to be consistent with JSObject::putDirectInternal, but I almost feel like putting it inside Structure::addPropertyWithoutTransition is a more natural place for it.
Comment on attachment 283228 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=283228&action=review >> Source/JavaScriptCore/runtime/JSObject.h:1569 >> + structure->setContainsReadOnlyProperties(); > > I put this here to be consistent with JSObject::putDirectInternal, > but I almost feel like putting it inside Structure::addPropertyWithoutTransition > is a more natural place for it. I think that makes sense in addPropertyWithoutTransition() for both call sites. What's wrong with that?
Comment on attachment 283228 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=283228&action=review >>> Source/JavaScriptCore/runtime/JSObject.h:1569 >>> + structure->setContainsReadOnlyProperties(); >> >> I put this here to be consistent with JSObject::putDirectInternal, >> but I almost feel like putting it inside Structure::addPropertyWithoutTransition >> is a more natural place for it. > > I think that makes sense in addPropertyWithoutTransition() for both call sites. What's wrong with that? I agree. I'm going to open another bug to consider this. I think we should also consider the Structure::addNewPropertyTransition to have it mark the bit on the new transition it's returning.
I opened: https://bugs.webkit.org/show_bug.cgi?id=159593
Comment on attachment 283228 [details] patch Clearing flags on attachment: 283228 Committed r203015: <http://trac.webkit.org/changeset/203015>
All reviewed patches have been landed. Closing bug.