[webkit-reviews] review requested: [Bug 27771] Web Inspector: Reimplement Elements Panel so that its interaction with DOM is serialized : [Attachment 33936] InspectorDOMAgent and WebInspector.DOMAgent

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Aug 1 11:03:24 PDT 2009

Pavel Feldman <pfeldman at chromium.org> has asked  for review:
Bug 27771: Web Inspector: Reimplement Elements Panel so that its interaction
with DOM is serialized

Attachment 33936: InspectorDOMAgent and WebInspector.DOMAgent

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
(In reply to comment #5)
> (From update of attachment 33935 [details])
> I thought the payload was going to use objects with properties and not an
> array. It seems fragile to use an array like this. Maybe they are arrays to
> minimize the serialized size?
> If we continue to use an array, this should have a comment about having the
> indexes stay in sync with the C++ file and visa versa. Also these names
> not be all caps (should be TitleCase).

It was done to minimize serialized size. But let us try dictionaries first and
see if it is actually slow. Updated with dictionaries.

> The underscore should be a prefix.


> This is another place using an array is fragile. It this was an object with
> properties you could name each item without the need for a comment and not
> to worry about it breaking if somthing changes. Also why not just use
> Node.DOCUMENT_NODE instead of 9?

Done. (using dictionary and Node.DOCUMENT_NODE).

> Having this here seems wrong. I assume this is temporary? Maybe add a

Yes. Commented and added TODO
> Temporary?

Yes. Added TODO.

> > +	 // Whitespace is ignored in InspectorDOMAgent already -> no need to
> > +	 Preferences.ignoreWhitespace = false;
> I guess we can remove this and all the users once we make the switch.

Yes. Added TODO
> > +	 this.__disabledProperties = payload[3];
> > +	 this.__disabledPropertyValues = payload[4];
> > +	 this.__disabledPropertyPriorities = payload[5];
> These should just have single underscore prefixes. The arrays still make a me

> nervous.

Not using arrays anymore, but am using __disabledProperties since we feed
StylesSidebarPane with it and latter relies upon this name.

More information about the webkit-reviews mailing list