Bug 34282

Summary: Simplify anonymous slot implementation
Product: WebKit Reporter: Oliver Hunt <oliver>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Mac OS X 10.5   
Attachments:
Description Flags
Patch barraclough: review+

Description Oliver Hunt 2010-01-28 14:02:15 PST
Simplify anonymous slot implementation
Comment 1 Oliver Hunt 2010-01-28 14:10:05 PST
Created attachment 47646 [details]
Patch
Comment 2 Gavin Barraclough 2010-01-28 14:21:17 PST
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 3 Darin Adler 2010-01-28 14:29:55 PST
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
Comment 4 Oliver Hunt 2010-01-28 14:51:18 PST
Committed r54022: <http://trac.webkit.org/changeset/54022>
Comment 5 Adam Barth 2010-01-28 16:52:23 PST
This patch caused the qt-ews to continually fail to upload results to QueueStatusServer.  Not sure why.