Bug 101547 – REGRESSION(r133840): Multiple selection repaint tests (e.g. fast/repaint/japanese-rl-selection-clear.html) fail

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Hayato Ito]

The following layout tests are failing on chromium canary.

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Frepaint%2Fjapanese-rl-selection-clear.html%2Cfast%2Frepaint%2Fjapanese-rl-selection-repaint-in-regions.html%2Cfast%2Frepaint%2Finline-outline-repaint.html%2Cfast%2Frepaint%2F4774354.html%2Cfast%2Frepaint%2Fdelete-into-nested-block.html%2Cfast%2Frepaint%2Frepaint-across-writing-mode-boundary.html%2Cfast%2Frepaint%2Fselection-rl.html%2Cfast%2Frepaint%2Fjapanese-rl-selection-repaint.html%2Cfast%2Frepaint%2Fcaret-outside-block.html

Probable cause:

r133840

I've ping-ed rniwa in #23bkit irc. He will take a look at that.

[Hayato Ito]

s/#23bkit/#webkit/

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

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.

[Joshua Bell]

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

[Joshua Bell]

I suppressed these failures in http://trac.webkit.org/changeset/133917 - thanks and good luck, rniwa!

[Ryosuke Niwa]

Created attachment 173250 [details]
Fixes the regression

[Build Bot]

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

[Ryosuke Niwa]

(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.

[Joshua Bell]

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>*)

[Ryosuke Niwa]

(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.

[WebKit Review Bot]

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

[Ryosuke Niwa]

Created attachment 173494 [details]
Patch for landing

[WebKit Review Bot]

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

[Ryosuke Niwa]

Committed r134191: <http://trac.webkit.org/changeset/134191>

[Chris Dumez]

This patch appears to be causing a few crashes on the WebKit2 EFL bot:

Backtraces:
http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r134192%20%285765%29/fast/overflow/scroll-div-hide-show-crash-log.txt

http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r134192%20%285765%29/fast/frames/seamless/seamless-sandbox-flag-crash-log.txt

http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r134192%20%285765%29/fast/frames/flattening/iframe-flattening-nested-crash-log.txt

http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r134192%20%285765%29/media/track/track-cues-sorted-before-dispatch-crash-log.txt

http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r134192%20%285765%29/media/video-controls-captions-crash-log.txt

[Chris Dumez]

(In reply to comment #15)
> This patch appears to be causing a few crashes on the WebKit2 EFL bot:
> 
> Backtraces:
> http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r134192%20%285765%29/fast/overflow/scroll-div-hide-show-crash-log.txt
> 
> http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r134192%20%285765%29/fast/frames/seamless/seamless-sandbox-flag-crash-log.txt
> 
> http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r134192%20%285765%29/fast/frames/flattening/iframe-flattening-nested-crash-log.txt
> 
> http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r134192%20%285765%29/media/track/track-cues-sorted-before-dispatch-crash-log.txt
> 
> http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r134192%20%285765%29/media/video-controls-captions-crash-log.txt

Actually, same thing on WK2 Debug Apple bot:
http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20%28Tests%29/r134191%20%283458%29/results.html

[Ryosuke Niwa]

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...

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

Build fix landed in http://trac.webkit.org/changeset/134197.