[webkit-reviews] review denied: [Bug 171016] Implement the imageready event to reliably test the async image decoding : [Attachment 308923] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 171016: Implement the imageready event to reliably test the async image
decoding
https://bugs.webkit.org/show_bug.cgi?id=171016

Attachment 308923: Patch

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




--- Comment #35 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 308923
  --> https://bugs.webkit.org/attachment.cgi?id=308923
Patch

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

> Source/WTF/wtf/FeatureDefines.h:769
> +#if !defined(ENABLE_IMAGE_READY_EVENT)
> +#define ENABLE_IMAGE_READY_EVENT 1
> +#endif

Remove this.

> Source/WebCore/dom/Element.cpp:2516
> +	   dispatchEvent(Event::create(eventNames().imagereadyEvent, true,
true));

I think this can just be dispatchEvent(Event::create("webkitImageFrameReady",
true, true));

> Source/WebCore/dom/Element.h:494
> +    void dispatchImageReadyEvent();

Rename with "ForTesting"

> Source/WebCore/dom/EventNames.h:249
> +    macro(imageready) \

Remove.

> Source/WebCore/page/RuntimeEnabledFeatures.h:219
> +#if ENABLE(IMAGE_READY_EVENT)
> +    void setImageReadyEventEnabled(bool isEnabled) {
m_isImageReadyEventEnabled = isEnabled; }
> +    bool imageReadyEventEnabled() const { return m_isImageReadyEventEnabled;
}
> +#endif

This becomes a Setting: webkitImageReadyEventEnabled

> Source/WebCore/testing/InternalSettings.cpp:114
> +#if ENABLE(IMAGE_READY_EVENET)
> +    ,
m_imageReadyEventEnabled(RuntimeEnabledFeatures::sharedFeatures().imageReadyEve
ntEnabled())
> +#endif
>  {

Remove.

> Source/WebCore/testing/InternalSettings.cpp:204
> +#if ENABLE(IMAGE_READY_EVENET)
> +   
RuntimeEnabledFeatures::sharedFeatures().setImageReadyEventEnabled(m_imageReady
EventEnabled);
> +#endif

Remove.

> Source/WebCore/testing/InternalSettings.cpp:727
> +void InternalSettings::setImageReadyEventEnabled(bool enabled)
> +{
> +#if ENABLE(IMAGE_READY_EVENT)
> +   
RuntimeEnabledFeatures::sharedFeatures().setImageReadyEventEnabled(enabled);
> +#else
> +    UNUSED_PARAM(enabled);
> +#endif
> +}

Remove.

> Source/WebCore/testing/InternalSettings.h:120
> +    static void setImageReadyEventEnabled(bool);

Remove.

> Source/WebCore/testing/InternalSettings.h:198
> +	   bool m_imageReadyEventEnabled;

Remove

> Source/WebCore/testing/InternalSettings.idl:90
> +    void setImageReadyEventEnabled(boolean enabled);

Remove.

> Tools/DumpRenderTree/DumpRenderTree.h:57
> +enum class DisplayOptions {
> +    ContentsAndTrackRepaints,
> +    ContentsOnly
> +};

Let's move this to a separate patch.