Bug 62130

Created attachment 96113 [details] Patch

Comment on attachment 96113 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96113&action=review > Source/WebCore/dom/Node.cpp:842 > - renderer()->absoluteRects(rects, absPos.x(), absPos.y()); > + renderer()->absoluteRects(rects, flooredIntPoint(absPos)); Confused. Is that what the old behavior did? > Source/WebCore/rendering/RenderBlock.cpp:5692 > + continuation()->absoluteRects(rects, accumulatedOffset - toSize(location() + > + inlineElementContinuation()->containingBlock()->location())); I might have broken this out into a local IntPOint variable. But looks fine. > Source/WebCore/rendering/RenderInline.cpp:474 > + rects.append(enclosingIntRect(FloatRect(accumulatedOffset + IntSize(curr->x(), curr->y()), IntSize(curr->width(), curr->height())))); curr->location()? curr->size()? > Source/WebCore/rendering/RenderText.cpp:275 > + rects.append(enclosingIntRect(FloatRect(accumulatedOffset + IntSize(box->x(), box->y()), IntSize(box->x(), box->y())))); you meant box width/height.

(In reply to comment #2) > > Source/WebCore/dom/Node.cpp:842 > > - renderer()->absoluteRects(rects, absPos.x(), absPos.y()); > > + renderer()->absoluteRects(rects, flooredIntPoint(absPos)); > > Confused. Is that what the old behavior did? Yeah, before there was an implicit cast from float to int, now it's explicit. > > Source/WebCore/rendering/RenderInline.cpp:474 > > + rects.append(enclosingIntRect(FloatRect(accumulatedOffset + IntSize(curr->x(), curr->y()), IntSize(curr->width(), curr->height())))); > > curr->location()? curr->size()? Sadly we don't have InlineBox::size yet. Changed x, y to topLeft which is the location equivalent for InlineBox. > > > Source/WebCore/rendering/RenderText.cpp:275 > > + rects.append(enclosingIntRect(FloatRect(accumulatedOffset + IntSize(box->x(), box->y()), IntSize(box->x(), box->y())))); > > you meant box width/height. Good catch, thank you!

Created attachment 96129 [details] Patch

Comment on attachment 96129 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96129&action=review > Source/WebCore/rendering/RenderInline.cpp:474 > + rects.append(enclosingIntRect(FloatRect(accumulatedOffset + curr->topLeft(), IntSize(curr->width(), curr->height())))); It might be worth adding a InlineBox::size(), even just for thsi one caller. > Source/WebCore/rendering/RenderText.cpp:275 > + rects.append(enclosingIntRect(FloatRect(accumulatedOffset + box->topLeft(), IntSize(box->width(), box->height())))); two callers. :)

> It might be worth adding a InlineBox::size(), even just for thsi one caller. > ... > two callers. :) Good point, will fix before landing. Thanks for the review Eric!

Created attachment 96141 [details] Patch for landing

The commit-queue encountered the following flaky tests while processing attachment 96141 [details]: http/tests/websocket/tests/frame-length-overflow.html bug 61507 (author: abarth@webkit.org) The commit-queue is continuing to process your patch.

Comment on attachment 96141 [details] Patch for landing Clearing flags on attachment: 96141 Committed r88202: <http://trac.webkit.org/changeset/88202>

All reviewed patches have been landed. Closing bug.

This broke the clang build thusly: In file included from third_party/WebKit/Source/WebCore/rendering/svg/RenderSVGContainer.h:28: third_party/WebKit/Source/WebCore/rendering/svg/RenderSVGModelObject.h:58:18:error: 'WebCore::RenderSVGModelObject::absoluteRects' hides overloaded virtual function [-Werror,-Woverloaded-virtual] virtual void absoluteRects(Vector<IntRect>&, int tx, int ty); ^ third_party/WebKit/Source/WebCore/rendering/RenderObject.h:593:18: note: hidden overloaded virtual function 'WebCore::RenderObject::absoluteRects' declared here virtual void absoluteRects(Vector<IntRect>&, const IntPoint&) { } ^ Please be careful when doing refactorings. We have a chromium bot upstream (the clang bot) that catches some of the problems (as in this case), but it doesn't catch everything.

Thanks Nico! I'll be more careful.

Reopening as the patch got reverted.

Created attachment 96296 [details] Patch

Comment on attachment 96296 [details] Patch Clearing flags on attachment: 96296 Committed r88297: <http://trac.webkit.org/changeset/88297>

All reviewed patches have been landed. Closing bug.