Summary: | Simplify anonymous slot implementation | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Oliver Hunt <oliver> | ||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Oliver Hunt
2010-01-28 14:02:15 PST
Created attachment 47646 [details]
Patch
Comment on attachment 47646 [details]
Patch
The hard coded ", 0" in JSCel / API wrappers is a little weird to me, I'd move AnonymousSlotCount up from JSValue personally. Your call. r+.
Comment on attachment 47646 [details] Patch > PassRefPtr<Structure> JSByteArray::createStructure(JSValue prototype) > { > - PassRefPtr<Structure> result = Structure::create(prototype, TypeInfo(ObjectType, StructureFlags)); > + PassRefPtr<Structure> result = Structure::create(prototype, TypeInfo(ObjectType, StructureFlags), AnonymousSlotCount); > return result; > } Why the local variable? Normally we don't use PassRefPtr for local variables, and normally we wouldn't use a local variable at all for this case. > + static const unsigned AnonymousSlotCount = 0; > protected: It's dangerous to have something like this as a public data member. It gives the wrong value if you use it on a base class of an object of a derived class. Can we make it protected instead? Maybe we can name it InitialAnonymousSlotCount too. > Structure::~Structure() > { > if (m_previous) { > - if (m_nameInPrevious) > - m_previous->table.remove(make_pair(m_nameInPrevious.get(), m_attributesInPrevious), m_specificValueInPrevious); > - else > - m_previous->table.removeAnonymousSlotTransition(m_anonymousSlotsInPrevious); > + ASSERT(m_nameInPrevious); > + m_previous->table.remove(make_pair(m_nameInPrevious.get(), m_attributesInPrevious), m_specificValueInPrevious); > > } You should remove that extra blank line. r=me Committed r54022: <http://trac.webkit.org/changeset/54022> This patch caused the qt-ews to continually fail to upload results to QueueStatusServer. Not sure why. |