Bug 34282

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

Description From 2010-01-28 14:02:15 PST
Simplify anonymous slot implementation
------- Comment #1 From 2010-01-28 14:10:05 PST -------
Created an attachment (id=47646) [details]
Patch
------- Comment #2 From 2010-01-28 14:21:17 PST -------
(From update of attachment 47646 [details])
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 From 2010-01-28 14:29:55 PST -------
(From update of attachment 47646 [details])
>  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 From 2010-01-28 14:51:18 PST -------
Committed r54022: <http://trac.webkit.org/changeset/54022>
------- Comment #5 From 2010-01-28 16:52:23 PST -------
This patch caused the qt-ews to continually fail to upload results to QueueStatusServer.  Not sure why.