[Webkit-unassigned] [Bug 93443] Web Inspector: Protocol Extension: Add "regionLayoutUpdate" event

[bugzilla-daemon at webkit.org]

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


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #160934|review?                     |review-
               Flag|                            |




--- Comment #10 from Pavel Feldman <pfeldman at chromium.org>  2012-08-31 02:08:37 PST ---
(From update of attachment 160934)
View in context: https://bugs.webkit.org/attachment.cgi?id=160934&action=review

Overall looks good. A number of renaming suggestions inline.

> Source/WebCore/dom/NamedFlowCollection.cpp:98
> +    InspectorInstrumentation::didRemoveNamedFlow(m_document, namedFlow);

willRemoveNamedFlow? (it is now happening before the actual removal)

> Source/WebCore/inspector/Inspector.json:2446
> +                "name": "regionLayoutUpdate",

regionLayoutUpdated?

> Source/WebCore/inspector/InspectorCSSAgent.cpp:542
> +    int documentNodeId = m_domAgent->boundNodeId(document);

Sounds like this is checked 3 times. A helper method documentNodeWithRequestedFlowsId?

> Source/WebCore/inspector/front-end/CSSStyleModel.js:381
> +        if (!this.hasEventListeners(WebInspector.CSSStyleModel.Events.NamedFlowCreated))

It is too late for this check, it does not save anything.

> Source/WebCore/inspector/front-end/CSSStyleModel.js:398
>          if (!this.hasEventListeners(WebInspector.CSSStyleModel.Events.NamedFlowRemoved))

ditto

> Source/WebCore/inspector/front-end/CSSStyleModel.js:415
> +        if (!this.hasEventListeners(WebInspector.CSSStyleModel.Events.RegionLayoutUpdate))

ditto

> Source/WebCore/inspector/front-end/CSSStyleModel.js:488
> +        this._namedFlowCollections = {};

Should this fire an event?

> Source/WebCore/inspector/front-end/CSSStyleModel.js:1436
> +    getFlowByName: function(flowName)

flowByName

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