Bug 137273 - Descendant is being put into the wrong flow thread with nested columns and spans
Summary: Descendant is being put into the wrong flow thread with nested columns and spans
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords: InRadar
: 137316 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-09-30 15:11 PDT by Dave Hyatt
Modified: 2015-02-16 10:07 PST (History)
15 users (show)

See Also:


Attachments
Patch (4.83 KB, patch)
2014-11-04 17:20 PST, Dave Hyatt
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2014-09-30 15:11:40 PDT
Descendant is being put into the wrong flow thread with nested columns and spans. See the FIXME in isValidColumnSpanner in RenderMultiColumnFlowThread.cpp.
Comment 1 Dave Hyatt 2014-11-04 17:20:46 PST
Created attachment 240986 [details]
Patch
Comment 2 Simon Fraser (smfr) 2014-11-05 09:11:58 PST
Comment on attachment 240986 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240986&action=review

> Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp:387
> +                placeholder.parent()->removeChild(placeholder);

I think you want a "// FIXME: placeholder is leaked" here.
Comment 3 David Kilzer (:ddkilzer) 2014-11-05 09:14:19 PST
Comment on attachment 240986 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240986&action=review

> Source/WebCore/ChangeLog:20
> +        The second fix was to stop destroying the placeholder. Since the placeholder can just have been inserted, you
> +        can't delete it, since otherwise code further up the stack will access the deleted object. For now, we just
> +        leak the placeholder.

Would using a RenderPtr<> for the placeholder (née descendant) fix the leak/object lifetime?

>> Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp:387
>> -                ancestorBlock.multiColumnFlowThread()->handleSpannerRemoval(spanner);
>> -                placeholder.destroy();
>> -                
>> +                if (subtreeRoot == descendant)
>> +                    subtreeRoot = spanner;
>> +                placeholder.parent()->removeChild(placeholder);
> 
> I think you want a "// FIXME: placeholder is leaked" here.

And a new bug number that tracks the leak please.
Comment 4 Dave Hyatt 2014-11-05 13:26:43 PST
Fixed in r175641.
Comment 5 Radar WebKit Bug Importer 2014-11-05 13:27:02 PST
<rdar://problem/18885189>
Comment 6 David Kilzer (:ddkilzer) 2015-02-03 15:49:39 PST
*** Bug 137316 has been marked as a duplicate of this bug. ***