[webkit-reviews] review denied: [Bug 39037] iframe/compositing propagation should also work for <object> and framesets : [Attachment 89029] Patch

Mon Apr 11 11:11:14 PDT 2011

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Chris Marrin
<cmarrin at apple.com>'s request for review:
Bug 39037: iframe/compositing propagation should also work for <object> and
framesets
https://bugs.webkit.org/show_bug.cgi?id=39037

Attachment 89029: Patch
https://bugs.webkit.org/attachment.cgi?id=89029&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=89029&action=review

The code changes look good, but the tests need improvement.

> Source/WebCore/ChangeLog:9
> +	   iframe/compositing propagation should also work for <object> and
framesets
> +	   https://bugs.webkit.org/show_bug.cgi?id=39037
> +
> +	   iframe/compositing propagation should also work for <object> and
framesets
> +	   https://bugs.webkit.org/show_bug.cgi?id=39037

Duplicated lines here.

> Source/WebCore/ChangeLog:11
> +	   This adds logic to handle <frame> and <object> just like <iframe>. I
moved

Logic for what?

> Source/WebCore/ChangeLog:18
> +	   some of the accelerated compositing tests from RenderIFrame to
RenderPart
> +	   which is the highest base class common to RenderIFrame, RenderFrame
and 
> +	   RenderEmbeddedObject. I also renamed several ...IFrame... functions
to
> +	   ...Frame... and changed them so they work on a RenderPart for
determining
> +	   if the renderer is composited. One other detail is that <iframe> and

> +	   <object> never share style since they might be composited. I had to
add
> +	   <frame> to that list.

These details would be better listed under the methods affected in the list of
changed files below.

> LayoutTests/compositing/framesets/composited-frame-alignment.html:1
> +<!DOCTYPE html>

I think these tests should dump the layer tree (in which case you'll have to
either trick the tests to give the same output in WK1 and WK2, or provide
separate results).

Also, instead of a textual description of what the result should look like,
just use red/green boxes to indicate success.