Bug 101547 - REGRESSION(r133840): Multiple selection repaint tests (e.g. fast/repaint/japanese-rl-selection-clear.html) fail
: REGRESSION(r133840): Multiple selection repaint tests (e.g. fast/repaint/japa...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Tools / Tests
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Ryosuke Niwa
:
Depends on: 101528 101576
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-07 23:11 PST by Hayato Ito
Modified: 2012-11-12 01:35 PST (History)
10 users (show)

See Also:


Attachments
Fixes the regression (9.60 KB, patch)
2012-11-09 02:51 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch for landing (9.85 KB, patch)
2012-11-11 00:50 PST, Ryosuke Niwa
webkit.review.bot: commit‑queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Hayato Ito 2012-11-07 23:12:21 PST
s/#23bkit/#webkit/
Comment 2 Ryosuke Niwa 2012-11-07 23:23:27 PST
For other contributors, please don't roll out this patch. It fixes a serious security vulnerability at the cost of possibly triggering an extra layout. The assertion failure we're seeing is an indication of another security bug, which we need to fix ASAP. I'm intending to work on this tonight.
Comment 3 Ryosuke Niwa 2012-11-08 03:19:01 PST
I've uploaded a fix for assertion failures on the bug 101576. I'm not sure what's up with the repaint tests. I'll investigate it tomorrow.
Comment 4 Joshua Bell 2012-11-08 10:28:24 PST
Just so it's not lost, the full list of failing tests is:

Image:
fast/repaint/selection-after-delete.html
fast/repaint/delete-into-nested-block.html
svg/custom/hit-test-unclosed-subpaths.svg
fast/repaint/no-caret-repaint-in-non-content-editable-element.html
fast/repaint/caret-outside-block.html
fast/repaint/4774354.html
fast/repaint/repaint-across-writing-mode-boundary.html
fast/repaint/japanese-rl-selection-clear.html
fast/repaint/selection-rl.html
fast/repaint/japanese-rl-selection-repaint.html
fast/repaint/inline-outline-repaint.html
fast/repaint/japanese-rl-selection-repaint-in-regions.html
svg/custom/foreignObject-crash-on-hover.xml
svg/custom/hit-test-with-br.xhtml

Crash - ASSERTION FAILED: !v->needsLayout():
editing/selection/focus_editable_html.html
editing/selection/click-after-nested-block.html
fast/events/autoscroll.html
Comment 5 Joshua Bell 2012-11-08 10:38:04 PST
I suppressed these failures in http://trac.webkit.org/changeset/133917 - thanks and good luck, rniwa!
Comment 6 Ryosuke Niwa 2012-11-09 02:51:32 PST
Created attachment 173250 [details]
Fixes the regression
Comment 7 Build Bot 2012-11-09 05:49:44 PST
Comment on attachment 173250 [details]
Fixes the regression

Attachment 173250 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14781260

New failing tests:
fast/frames/flattening/iframe-flattening-fixed-height.html
fast/frames/flattening/frameset-flattening-advanced.html
fast/frames/flattening/frameset-flattening-subframesets.html
fast/frames/flattening/iframe-flattening-out-of-view.html
fast/frames/flattening/frameset-flattening-grid.html
fast/frames/flattening/iframe-flattening-fixed-width.html
fast/frames/flattening/iframe-flattening-simple.html
Comment 8 Ryosuke Niwa 2012-11-09 14:22:15 PST
(In reply to comment #7)
> (From update of attachment 173250 [details])
> Attachment 173250 [details] did not pass mac-ews (mac):
> Output: http://queues.webkit.org/results/14781260
> 
> New failing tests:
> fast/frames/flattening/iframe-flattening-fixed-height.html
> fast/frames/flattening/frameset-flattening-advanced.html
> fast/frames/flattening/frameset-flattening-subframesets.html
> fast/frames/flattening/iframe-flattening-out-of-view.html
> fast/frames/flattening/frameset-flattening-grid.html
> fast/frames/flattening/iframe-flattening-fixed-width.html
> fast/frames/flattening/iframe-flattening-simple.html

This appears to be false positives.
Comment 9 Joshua Bell 2012-11-09 14:28:08 PST
FYI, editing/input/ime-composition-clearpreedit.html started flakily crashing with the same assert:

http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.7%20(dbg)/builds/633/steps/webkit_tests/logs/stdio

10:44:19.986 1112 worker/5 editing/input/ime-composition-clearpreedit.html crashed, (stderr lines):
10:44:19.986 1112   ASSERTION FAILED: !v->needsLayout()
10:44:19.986 1112   /Volumes/data/b/build/slave/webkit-mac-latest-dbg/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../editing/FrameSelection.cpp(1328) : bool WebCore::FrameSelection::recomputeCaretRect()
10:44:19.986 1112   1   0x27bd9f3 WebCore::FrameSelection::recomputeCaretRect()
10:44:19.986 1112   2   0x27be157 WebCore::FrameSelection::invalidateCaretRect()
10:44:19.986 1112   3   0x27b3bb2 WebCore::FrameSelection::caretBlinkTimerFired(WebCore::Timer<WebCore::FrameSelection>*)
Comment 10 Ryosuke Niwa 2012-11-09 14:33:17 PST
(In reply to comment #9)
> FYI, editing/input/ime-composition-clearpreedit.html started flakily crashing with the same assert:
> 
> http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.7%20(dbg)/builds/633/steps/webkit_tests/logs/stdio
> 
> 10:44:19.986 1112 worker/5 editing/input/ime-composition-clearpreedit.html crashed, (stderr lines):
> 10:44:19.986 1112   ASSERTION FAILED: !v->needsLayout()
> 10:44:19.986 1112   /Volumes/data/b/build/slave/webkit-mac-latest-dbg/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../editing/FrameSelection.cpp(1328) : bool WebCore::FrameSelection::recomputeCaretRect()
> 10:44:19.986 1112   1   0x27bd9f3 WebCore::FrameSelection::recomputeCaretRect()
> 10:44:19.986 1112   2   0x27be157 WebCore::FrameSelection::invalidateCaretRect()
> 10:44:19.986 1112   3   0x27b3bb2 WebCore::FrameSelection::caretBlinkTimerFired(WebCore::Timer<WebCore::FrameSelection>*)

Oh, no :( Yet another bug.
Comment 11 WebKit Review Bot 2012-11-11 00:23:41 PST
Comment on attachment 173250 [details]
Fixes the regression

Rejecting attachment 173250 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
editing/FrameSelection.h
patching file Source/WebCore/loader/FrameLoader.cpp
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/chromium/TestExpectations
Hunk #2 FAILED at 4269.
1 out of 2 hunks FAILED -- saving rejects to file LayoutTests/platform/chromium/TestExpectations.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Simon Fras..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/14675515
Comment 12 Ryosuke Niwa 2012-11-11 00:50:41 PST
Created attachment 173494 [details]
Patch for landing
Comment 13 WebKit Review Bot 2012-11-11 02:02:39 PST
Comment on attachment 173494 [details]
Patch for landing

Rejecting attachment 173494 [details] from commit-queue.

New failing tests:
fast/canvas/canvas-resize-reset-pixelRatio.html
platform/chromium/virtual/gpu/fast/canvas/canvas-as-image-hidpi.html
fast/canvas/canvas-as-image-hidpi.html
platform/chromium/virtual/gpu/fast/canvas/canvas-resize-reset-pixelRatio.html
Full output: http://queues.webkit.org/results/14778990
Comment 14 Ryosuke Niwa 2012-11-11 23:10:07 PST
Committed r134191: <http://trac.webkit.org/changeset/134191>
Comment 17 Ryosuke Niwa 2012-11-12 01:21:25 PST
Apparently rendering code can call updateAppearance in the middle of a layout :(

STDERR: ASSERTION FAILED: !view() || (!view()->isInLayout() && !view()->isPainting())
STDERR: /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/Source/WebCore/dom/Document.cpp(1898) : void WebCore::Document::updateStyleIfNeeded()
STDERR: 1   0x7fce91fea5ce WebCore::Document::updateStyleIfNeeded()
STDERR: 2   0x7fce91fea7e2 WebCore::Document::updateLayout()
STDERR: 3   0x7fce91fea7d6 WebCore::Document::updateLayout()
STDERR: 4   0x7fce92159d6e WebCore::FrameSelection::updateAppearance()
STDERR: 5   0x7fce9255af2e WebCore::FrameView::performPostLayoutTasks()
STDERR: 6   0x7fce925568e2 WebCore::FrameView::layout(bool)
STDERR: 7   0x7fce928ef45f WebCore::RenderWidget::updateWidgetPosition()
STDERR: 8   0x7fce928e5125 WebCore::RenderView::updateWidgetPositions()
STDERR: 9   0x7fce9281976e WebCore::RenderLayer::scrollTo(int, int)
STDERR: 10  0x7fce9281bff9 WebCore::RenderLayer::setScrollOffset(WebCore::IntPoint const&)
STDERR: 11  0x7fce925f38ae WebCore::ScrollableArea::scrollPositionChanged(WebCore::IntPoint const&)
STDERR: 12  0x7fce925f3b49 WebCore::ScrollableArea::setScrollOffsetFromAnimation(WebCore::IntPoint const&)
STDERR: 13  0x7fce925f31f9 WebCore::ScrollAnimator::notifyPositionChanged()
STDERR: 14  0x7fce925f2b8d WebCore::ScrollAnimator::scrollToOffsetWithoutAnimation(WebCore::FloatPoint const&)
STDERR: 15  0x7fce925f3744 WebCore::ScrollableArea::scrollToOffsetWithoutAnimation(WebCore::FloatPoint const&)
STDERR: 16  0x7fce928195ba WebCore::RenderLayer::scrollToOffset(WebCore::IntSize const&, WebCore::RenderLayer::ScrollOffsetClamping)
STDERR: 17  0x7fce9281f3ff WebCore::RenderLayer::updateScrollInfoAfterLayout()
STDERR: 18  0x7fce92730d74 WebCore::RenderBlock::updateScrollInfoAfterLayout()
STDERR: 19  0x7fce92731dd6 WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit)
STDERR: 20  0x7fce92730dee WebCore::RenderBlock::layout()
STDERR: 21  0x7fce92736a91 WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&)
STDERR: 22  0x7fce927365ec WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&)
STDERR: 23  0x7fce92731880 WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit)
STDERR: 24  0x7fce92730dee WebCore::RenderBlock::layout()
STDERR: 25  0x7fce92736a91 WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&)
STDERR: 26  0x7fce927365ec WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&)
STDERR: 27  0x7fce92731880 WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit)
STDERR: 28  0x7fce92730dee WebCore::RenderBlock::layout()
STDERR: 29  0x7fce92736a91 WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&)
STDERR: 30  0x7fce927365ec WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&)
STDERR: 31  0x7fce92731880 WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit)

I don't know why this didn't come up when I was running tests locally...
Comment 18 Ryosuke Niwa 2012-11-12 01:23:05 PST
I'll move updateLayout() code outside of updateAppearance and remove the assertions we've added in updateAppearance and recomputeCaretRect. Unfortunately, there aren't many options here.
Comment 19 Ryosuke Niwa 2012-11-12 01:35:18 PST
Build fix landed in http://trac.webkit.org/changeset/134197.