106075 – [CSS Regions][Mac] fast/regions/full-screen-video-from-region.html hits an assertion in RenderFlowThread::removeRenderBoxRegionInfo

RESOLVED FIXED 106075

[CSS Regions][Mac] fast/regions/full-screen-video-from-region.html hits an assertion in RenderFlowThread::removeRenderBoxRegionInfo

https://bugs.webkit.org/show_bug.cgi?id=106075

Summary [CSS Regions][Mac] fast/regions/full-screen-video-from-region.html hits an as...

Ryosuke Niwa

Reported 2013-01-03 19:18:59 PST

The test added by http://trac.webkit.org/changeset/138755 is hitting an assertion: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=full-screen-video-from-region.html http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r138769%20(4286)/results.html 0 com.apple.WebCore 0x000000010f55615b WebCore::RenderFlowThread::removeRenderBoxRegionInfo(WebCore::RenderBox*) + 459 (RenderFlowThread.cpp:449) 1 com.apple.WebCore 0x000000010f555f63 WebCore::RenderFlowThread::removeFlowChildInfo(WebCore::RenderObject*) + 67 (RenderFlowThread.cpp:90) 2 com.apple.WebCore 0x000000010f61b0b2 WebCore::RenderObject::removeFromRenderFlowThreadRecursive(WebCore::RenderFlowThread*) + 130 (RenderObject.cpp:2496) 3 com.apple.WebCore 0x000000010f61b093 WebCore::RenderObject::removeFromRenderFlowThreadRecursive(WebCore::RenderFlowThread*) + 99 (RenderObject.cpp:2493) 4 com.apple.WebCore 0x000000010f61b093 WebCore::RenderObject::removeFromRenderFlowThreadRecursive(WebCore::RenderFlowThread*) + 99 (RenderObject.cpp:2493) 5 com.apple.WebCore 0x000000010f61b093 WebCore::RenderObject::removeFromRenderFlowThreadRecursive(WebCore::RenderFlowThread*) + 99 (RenderObject.cpp:2493) 6 com.apple.WebCore 0x000000010f61b028 WebCore::RenderObject::removeFromRenderFlowThread() + 120 (RenderObject.cpp:2488) 7 com.apple.WebCore 0x000000010f61af52 WebCore::RenderObject::willBeRemovedFromTree() + 354 (RenderObject.cpp:2470) 8 com.apple.WebCore 0x000000010f61f3c5 WebCore::RenderObjectChildList::removeChildNode(WebCore::RenderObject*, WebCore::RenderObject*, bool) + 389 (RenderObjectChildList.cpp:92) 9 com.apple.WebCore 0x000000010f60be54 WebCore::RenderObject::removeChild(WebCore::RenderObject*) + 164 (RenderObject.cpp:343) 10 com.apple.WebCore 0x000000010f4653bb WebCore::RenderBlock::removeChild(WebCore::RenderObject*) + 1019 (RenderBlock.cpp:1195) 11 com.apple.WebCore 0x000000010f56a966 WebCore::RenderObject::remove() + 70 (RenderObject.h:936) 12 com.apple.WebCore 0x000000010f56a259 WebCore::RenderFullScreen::wrapRenderer(WebCore::RenderObject*, WebCore::RenderObject*, WebCore::Document*) + 425 (RenderFullScreen.cpp:132) 13 com.apple.WebCore 0x000000010e600496 WebCore::Document::webkitWillEnterFullScreenForElement(WebCore::Element*) + 646 (Document.cpp:5212) 14 com.apple.WebKit 0x000000010dc77ca6 -[WebKitFullScreenListener webkitWillEnterFullScreen] + 118 (WebKitFullScreenListener.mm:49) 15 DumpRenderTree 0x000000010c888477 -[UIDelegate enterFullScreenWithListener:] + 39 (UIDelegate.mm:261) 16 com.apple.Foundation 0x00007fff86ffbdb5 __NSFireDelayedPerform + 358 17 com.apple.CoreFoundation 0x00007fff8a050da4 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20 18 com.apple.CoreFoundation 0x00007fff8a0508bd __CFRunLoopDoTimer + 557 19 com.apple.CoreFoundation 0x00007fff8a036099 __CFRunLoopRun + 1513 20 com.apple.CoreFoundation 0x00007fff8a0356b2 CFRunLoopRunSpecific + 290 21 com.apple.Foundation 0x00007fff8702389e -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 268 22 DumpRenderTree 0x000000010c84b839 runTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 5017 (DumpRenderTree.mm:1381) 23 DumpRenderTree 0x000000010c84a42a runTestingServerLoop() + 282 (DumpRenderTree.mm:846) 24 DumpRenderTree 0x000000010c849cf7 dumpRenderTree(int, char const**) + 391 (DumpRenderTree.mm:893) 25 DumpRenderTree 0x000000010c84c029 main + 105 (DumpRenderTree.mm:931)

Attachments
Patch (10.39 KB, patch) 2013-01-31 08:54 PST, Andrei Bucur	webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Patch retry (10.39 KB, patch) 2013-01-31 13:31 PST, Andrei Bucur	no flags	Details Formatted Diff Diff
Patch with Zoltan's feedback (9.62 KB, patch) 2013-02-01 01:41 PST, Andrei Bucur	tony: review+	Details Formatted Diff Diff
Patch for landing (9.84 KB, patch) 2013-02-15 01:49 PST, Andrei Bucur	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Ryosuke Niwa

Comment 1 2013-01-03 19:21:53 PST

Added a test expectation in http://trac.webkit.org/changeset/138772.

Jessie Berlin

Comment 2 2013-01-30 15:27:13 PST

<rdar://problem/13114899>

Jessie Berlin

Comment 3 2013-01-30 15:29:37 PST

Rather(In reply to comment #2) > <rdar://problem/13114899> Rather, <rdar://problem/13003424>

Andrei Bucur

Comment 4 2013-01-31 08:54:13 PST

Created attachment 185794 [details] Patch

Zoltan Horvath

Comment 5 2013-01-31 09:20:24 PST

Comment on attachment 185794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185794&action=review It's cool that you figured this out! Just 2 small nits below. > Source/WebCore/ChangeLog:27 > + Tests: No new tests. Repaired fast/regions/full-screen-video-from-region.html Where is the fix? > LayoutTests/platform/qt/TestExpectations:-2557 > -webkit.org/b/106095 fast/regions/full-screen-video-from-region.html This is tracked in bug #106095 and it times out because Qt hasn't been implemented webkitRequestFullscreen() (bug #92078) yet.

WebKit Review Bot

Comment 6 2013-01-31 10:38:03 PST

Comment on attachment 185794 [details] Patch Attachment 185794 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16252504 New failing tests: fast/forms/datalist/update-range-with-datalist.html fast/layers/no-clipping-overflow-hidden-added-after-transform.html inspector-protocol/heap-profiler/heap-snapshot-with-detached-dom-tree.html fast/loader/text-document-wrapping.html fast/loader/javascript-url-in-object.html fast/layers/no-clipping-overflow-hidden-added-after-transition.html fast/layers/no-clipping-overflow-hidden-hardware-acceleration.html

Andrei Bucur

Comment 7 2013-01-31 13:24:35 PST

(In reply to comment #5) > (From update of attachment 185794 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=185794&action=review > > It's cool that you figured this out! Just 2 small nits below. > > > Source/WebCore/ChangeLog:27 > > + Tests: No new tests. Repaired fast/regions/full-screen-video-from-region.html > > Where is the fix? I meant I've fixed the assertion in debug mode :). > > > LayoutTests/platform/qt/TestExpectations:-2557 > > -webkit.org/b/106095 fast/regions/full-screen-video-from-region.html > > This is tracked in bug #106095 and it times out because Qt hasn't been implemented webkitRequestFullscreen() (bug #92078) yet. Oh, I thought that was added for the same reason. I'll fix it in the next version/before committing. Thanks for the review!

Andrei Bucur

Comment 8 2013-01-31 13:31:58 PST

Created attachment 185848 [details] Patch retry Retry the old patch to check for bad bots.

Andrei Bucur

Comment 9 2013-02-01 01:41:09 PST

Created attachment 185979 [details] Patch with Zoltan's feedback

Tony Chang

Comment 10 2013-02-14 10:31:57 PST

Comment on attachment 185979 [details] Patch with Zoltan's feedback View in context: https://bugs.webkit.org/attachment.cgi?id=185979&action=review > Source/WebCore/ChangeLog:11 > + When the region chain is invalidate the information is lost so we need to return true, even for the Grammar nit: "When the region chain is invalidated, the information is lost so ..." > Source/WebCore/ChangeLog:18 > + The second problem is RenderMedia not inheriting for RenderBlock. The logic of child relayout doesn't apply Nit: for -> from > Source/WebCore/rendering/RenderMedia.cpp:76 > + controlsRenderer->setNeedsLayout(true, MarkOnlyThis); Rather than calling setNeedsLayout, I would probably do something like, bool controlsNeedLayout = controlsRenderer->needsLayout(); if (inRenderFlowThread()) { ... controlsNeedLayout = true; } if (newSize == oldSize && !controlsNeedLayout) return;

Andrei Bucur

Comment 11 2013-02-15 01:49:45 PST

Created attachment 188513 [details] Patch for landing

WebKit Review Bot

Comment 12 2013-02-15 04:29:59 PST

Comment on attachment 188513 [details] Patch for landing Clearing flags on attachment: 188513 Committed r142982: <http://trac.webkit.org/changeset/142982>

Mihai Balan

Comment 13 2013-03-01 06:33:34 PST

Closing as the fix landed in r142982.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component CSS

Assignee

Andrei Bucur

Reported

2013-01-03 19:18 PST

Modified

2013-03-01 06:33 PST History

CC List

15 users Show

URL

Keywords AdobeTracked, InRadar, LayoutTestFailure

Depends on

104144

Blocks

57312

Dependencies

tree graph