Right now, InspectorOverlay::buildObjectForHighlightedNode and its helper methods manually build up InspectorObjects. This makes it really hard to tell what the shape of the data is without looking at the code that builds it. It also is a very thin abstraction between the code on the overlay page and its backend. I suggest we add a new Overlay domain that contains these types. This will give us type builders for free. At a later time, we could easily convert the InspectorOverlay::evaluateInOverlay calls into protocol commands.
<rdar://problem/16186822>
Created attachment 244843 [details] Patch
Comment on attachment 244843 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244843&action=review Looks good! There might be one issue with the ElementData change. > Source/WebCore/inspector/InspectorController.h:55 > +namespace Protocol { namespace OverlayTypes { > +class NodeHighlightData; > +} } > } Some newlines would help. (Unless this is the common style now.) > Source/WebCore/inspector/InspectorOverlay.cpp:719 > - elementInfo->setString("nodeWidth", String::number(modelObject ? adjustForAbsoluteZoom(modelObject->pixelSnappedOffsetWidth(), *modelObject) : boundingBox.width())); > - elementInfo->setString("nodeHeight", String::number(modelObject ? adjustForAbsoluteZoom(modelObject->pixelSnappedOffsetHeight(), *modelObject) : boundingBox.height())); > - > + auto sizeObject = Inspector::Protocol::OverlayTypes::Size::create() > + .setWidth(modelObject ? adjustForAbsoluteZoom(modelObject->pixelSnappedOffsetWidth(), *modelObject) : boundingBox.width()) > + .setHeight(modelObject ? adjustForAbsoluteZoom(modelObject->pixelSnappedOffsetHeight(), *modelObject) : boundingBox.height()) > + .release(); > + elementData->setSize(WTF::move(sizeObject)); Doesn't this require a change in the OverlayPage script? nodeWidth and nodeHeight is now size.width and size.height, right? > Source/WebCore/inspector/InspectorOverlay.cpp:870 > + .setDeviceScaleFactor(m_page.deviceScaleFactor()) I'm surprised we need to pass this along. > Source/WebCore/inspector/InspectorOverlay.h:49 > +namespace Protocol { > +namespace OverlayTypes { > +class NodeHighlightData; > +} > +} Like this.
Comment on attachment 244843 [details] Patch Never mind, I didn't see the InspectorOverlayPage.js changes on the first pass.
Created attachment 244846 [details] Patch
Created attachment 244847 [details] EWS Round 2
Created attachment 244851 [details] EWS Round 3
Committed r178631: <http://trac.webkit.org/changeset/178631>
(In reply to comment #8) > Committed r178631: <http://trac.webkit.org/changeset/178631> Includes windows and GTK build fixes.
(In reply to comment #8) > Committed r178631: <http://trac.webkit.org/changeset/178631> This broke the iOS build. Committed build fix in <http://trac.webkit.org/changeset/178636>.