[Webkit-unassigned] [Bug 63481] Bring V8's SerializedScriptValue implementation up to date

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=63481





--- Comment #24 from David Levin <levin at chromium.org>  2011-07-13 18:49:03 PST ---
(From update of attachment 100738)
View in context: https://bugs.webkit.org/attachment.cgi?id=100738&action=review

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:413
> +    void writeObjectOffset(uint32_t offset)

objectReference seems like the most commonly used term.
s/writeObjectOffset/writeObjectReference/

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:438
> +    void writeCheckRef(uint32_t numberOfRefs)

avoid abbreviations:s/Ref/Reference/ (for both instances).

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:440
> +        append(CheckRefTag);

Perhaps s/CheckRef/ReferenceCount/

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:560
> +        , m_nextObjectId(0)

objectReference seems like the most commonly used term, so

s/ObjectId/ObjectReference/

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:755
> +                // and is checked upon entry in Serializer::doSerialize.

I would get rid of the 2nd sentence (because it could easily get out of date).

I would consider making the 1st sentence a comment above where the member variable is declared.

// Used along with execDepth() to determine the number of
// accessors under which the serializer is currently serializing.
bool m_isSerializingAccessor;

(I changed the name here on purpose so that it clear that it is a bool and what question it answers.)

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:764
> +                // This m_serializingAccessor reset is necessary for those times when we are serializing

I don't think you need a comment for this case. (It is expected -- The odd thing is not reseting during a return. That is a typical buggy pattern so that comment is useful.)

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:964
> +        uint32_t objectId = m_nextObjectId++;

objectReference seems like the most commonly used term, so

s/objectId/objectReference/

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:1738
> +        uint32_t newId = m_objectPool.size();

objectReference seems like the most commonly used term, so

s/newId/newObjectReference/

(I'm not sure that we need "new" in this name. Up to you.)

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:1747
> +        uint32_t oldId = m_objectReferenceStack[m_objectReferenceStack.size() - 1];

objectReference seems like the most commonly used term, so

s/old/oldObjectReference/

(I'm not sure that we need "old" in this name. Up to you.)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.