If the viewport width equals the contents width, especially in the mobile device, TiledBackingStore endlessly creates and deletes the rightmost column and bottom row of tiles.
Created attachment 96687 [details] patch
Comment on attachment 96687 [details] patch Golly! We'll need this change in Source/WebKit2/UIProcess/TiledDrawingAreaProxy.cpp as well.
Created attachment 96723 [details] patch I also changed TiledDrawingAreaProxy. I'm glad to change WebKit2!
Jocelyn please look at this
Comment on attachment 96723 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=96723&action=review > Source/WebCore/ChangeLog:9 > + REGRESSION(410d71df): Fix Qt bustage. Please use SVN revision numbers, I guess you meant r77312. Please also say why it broke, that bottomRight used to do this - 1 correction itself and that it was replaced everywhere by maxX/Y, which doesn't anymore. I wonder, though, how this can be the only thing that broke, I don't understand. > Source/WebKit2/ChangeLog:9 > + REGRESSION(410d71df): Fix Qt bustage. Same. Otherwise looks fine, thanks for catching this!
Comment on attachment 96723 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=96723&action=review > Source/WebCore/platform/graphics/TiledBackingStore.cpp:80 > Tile::Coordinate topLeft = tileCoordinateForPoint(dirtyRect.location()); > - Tile::Coordinate bottomRight = tileCoordinateForPoint(IntPoint(dirtyRect.maxX(), dirtyRect.maxY())); > + Tile::Coordinate bottomRight = tileCoordinateForPoint(IntPoint(dirtyRect.maxX() - 1, dirtyRect.maxY() - 1)); > > for (unsigned yCoordinate = topLeft.y(); yCoordinate <= bottomRight.y(); ++yCoordinate) { > for (unsigned xCoordinate = topLeft.x(); xCoordinate <= bottomRight.x(); ++xCoordinate) { I think a bette fix would be to change the <= into a < rather than changing the value in bottomRight.
(In reply to comment #6) > (From update of attachment 96723 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96723&action=review > > > Source/WebCore/platform/graphics/TiledBackingStore.cpp:80 > > Tile::Coordinate topLeft = tileCoordinateForPoint(dirtyRect.location()); > > - Tile::Coordinate bottomRight = tileCoordinateForPoint(IntPoint(dirtyRect.maxX(), dirtyRect.maxY())); > > + Tile::Coordinate bottomRight = tileCoordinateForPoint(IntPoint(dirtyRect.maxX() - 1, dirtyRect.maxY() - 1)); > > > > for (unsigned yCoordinate = topLeft.y(); yCoordinate <= bottomRight.y(); ++yCoordinate) { > > for (unsigned xCoordinate = topLeft.x(); xCoordinate <= bottomRight.x(); ++xCoordinate) { > > I think a bette fix would be to change the <= into a < rather than changing the value in bottomRight. I don't like having to add the -1 everywhere either but the thing is that tileCoordinateForPoint translates the rect's coordinate by doing a int division by the tile size. So I might be missing something, but I think this would skip the last row/column of tiles in cases where dirtyRect is not on the edge of a tile and tileCoordinateForPoint(IntPoint(dirtyRect.maxX() - 1, dirtyRect.maxY() - 1)) == tileCoordinateForPoint(IntPoint(dirtyRect.maxX(), dirtyRect.maxY())).
Created attachment 96904 [details] It show why this patch is needed. To Darin Adler, '<=' is right on most case. However, if tile rect's width or height is power of n of cover(or dirty) rect's width or height, right bottom point is wrong. This picture explains it. To Jocelyn Turcotte, I agree that -1 everywhere is ugly. I will patch about it.
Created attachment 96932 [details] patch
Comment on attachment 96932 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=96932&action=review I am not a reviewer, bellow are some comments. > Source/WebCore/platform/graphics/TiledBackingStore.cpp:200 > +IntPoint TiledBackingStore::preciseBottomRight(const IntRect& rect) > +{ No need to make this a member, it should be a static function. > Source/WebKit2/ChangeLog:11 > + REGRESSION(r77928): Cause off-by-one error. The off-by-one error was introduced in r78783 and r78785. > Source/WebKit2/UIProcess/TiledDrawingAreaProxy.cpp:337 > +IntPoint TiledDrawingAreaTile::preciseBottomRight(const IntRect& rect) > +{ Same here. No need to make this a member, it should be a static function.
Comment on attachment 96932 [details] patch I'm not sure how "precision" has anything to do with this.
Created attachment 98285 [details] patch Changed innerBottomRight static function from preciseBottomRight member method.
Created attachment 98286 [details] patch
Comment on attachment 98286 [details] patch Any way to test this using a pixel test or timeout test (you mentioned an infinite loop)
I think it is difficult to make the test. The test requires to control tile size, test html content size, and window size. I have no idea about using Layout test to test this.
Comment on attachment 98286 [details] patch OK, but next time explain the lack of tests (and the reason why) in the changelog
OK.
Comment on attachment 98286 [details] patch Clearing flags on attachment: 98286 Committed r89803: <http://trac.webkit.org/changeset/89803>
All reviewed patches have been landed. Closing bug.