[webkit-reviews] review denied: [Bug 114496] [Texmap] Remove unnecessary TiledBackingStore code. : [Attachment 197921] Patch

[bugzilla-daemon at webkit.org]

Jocelyn Turcotte <jocelyn.turcotte at digia.com> has denied JungJik Lee
<jungjik.lee at samsung.com>'s request for review:
Bug 114496: [Texmap] Remove unnecessary TiledBackingStore code.
https://bugs.webkit.org/show_bug.cgi?id=114496

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

------- Additional Comments from Jocelyn Turcotte <jocelyn.turcotte at digia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=197921&action=review


> Source/WebCore/platform/graphics/TiledBackingStore.cpp:447
>	   IntRect expectedTileRect = tileRectForCoordinate(tileCoordinate);

The return value of tileRectForCoordinate is intersected with m_rect while, as
best as I can read the code, m_keepRect isn't.
This means that if the page's content size is shrunk, your patch wouldn't
remove tiles that fall outside the new content rect anymore since setKeepRect
doesn't handle this case.

I have to admit that TiledBackingStore is very badly tested and for this
reason, unless there is great gain in doing this kind of lossy refactoring, the
risk is usually not worth it.

r- assuming that what I'm saying is correct. Let me know if you think I'm
missing something.