[webkit-reviews] review denied: [Bug 198666] New Layout Test landed flaky [Mac WK1] resize-observer/element-leak.html is a flaky failure : [Attachment 371647] Patch

[bugzilla-daemon at webkit.org]

Frédéric Wang (:fredw) <fred.wang at free.fr> has denied cathiechen
<cathiechen at igalia.com>'s request for review:
Bug 198666: New Layout Test landed flaky [Mac WK1]
resize-observer/element-leak.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=198666

Attachment 371647: Patch

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




--- Comment #10 from Frédéric Wang (:fredw) <fred.wang at free.fr> ---
Comment on attachment 371647
  --> https://bugs.webkit.org/attachment.cgi?id=371647
Patch

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

> LayoutTests/ChangeLog:9
> +	   We don't need 1000 element to confirm the GC problem. So in order to
reduce the fake alarm.

1000 elements*
There should be a comma at the end of the line.
What is the "fake alarm", a timeout?

> LayoutTests/resize-observer/resources/element-leak-frame.html:18
> +var container = document.createElement('p');

p's content model is "phrasing content", so it is not valid to put a div
element inside.
https://html.spec.whatwg.org/multipage/grouping-content.html#the-p-element

You can make container a div instead.

> LayoutTests/resize-observer/resources/element-leak-frame.html:23
> +document.body.appendChild(container);

I don't know if this change is necessary, but it should be explained in the
ChangeLog.