Bug 106075

Summary:

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

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

CSS

Assignee:

Andrei Bucur <abucur>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abucur, achicu, dglazkov, eric.carlson, eric, feature-media-reviews, hyatt, jberlin, jer.noble, mibalan, mihnea, ojan.autocc, WebkitBugTracker, webkit.review.bot, zoltan

Priority:

Keywords:

AdobeTracked, InRadar, LayoutTestFailure

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

104144

Bug Blocks:

57312

Attachments:

Description	Flags
Patch	webkit.review.bot: commit-queue-
Patch retry	none
Patch with Zoltan's feedback	tony: review+
Patch for landing	none

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.