[webkit-reviews] review denied: [Bug 190159] Web Inspector: prevent layer events from firing until the layer information is re-requested : [Attachment 351304] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 1 15:06:40 PDT 2018


Joseph Pecoraro <joepeck at webkit.org> has denied Devin Rousso
<drousso at apple.com>'s request for review:
Bug 190159: Web Inspector: prevent layer events from firing until the layer
information is re-requested
https://bugs.webkit.org/show_bug.cgi?id=190159

Attachment 351304: Patch

https://bugs.webkit.org/attachment.cgi?id=351304&action=review




--- Comment #2 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 351304
  --> https://bugs.webkit.org/attachment.cgi?id=351304
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351304&action=review

> Source/WebCore/ChangeLog:8
> +	   No tests as this patch only has an effect when the layer information
isn't needed.

r- just because we should write a test for this because it would be pretty
important that we don't break it.

1. A test that triggers (layer change -> request -> layer change) and confirm 2
layer change events
2. A test that triggers (layer change -> no request -> layer change) and
confirm 1 layer change event

> Source/WebCore/inspector/agents/InspectorLayerTreeAgent.h:94
> +    bool m_layersRequested { true };

While the logic is right, this feels the opposite of what we want from a naming
convention. It seems unintuitive to start out by saying layers were requested.
How about any of these?

    m_suppressLayerChangeEvents { false };
    m_hasRequestedLayersSinceLastChange { false };


More information about the webkit-reviews mailing list