Bug 77099 - SVG + <object> tests are flakey
Summary: SVG + <object> tests are flakey
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
: 77061 (view as bug list)
Depends on:
Blocks: 68523 76447
  Show dependency treegraph
 
Reported: 2012-01-26 06:25 PST by Nikolas Zimmermann
Modified: 2012-01-26 07:06 PST (History)
4 users (show)

See Also:


Attachments
Patch (24.11 KB, patch)
2012-01-26 06:41 PST, Nikolas Zimmermann
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2012-01-26 06:25:00 PST
Bug 76447 changed the way RenderSVGRoot figures out its size. Previously RenderSVGRoot directly called out to the ownerRenderer (RenderEmbeddedObject) to compute its replaced size when embedded through eg. <object> element, which was quite hacky.
It now relies on the ownerRenderers availableLogicalWidth/Height to be correctly set, which requires that the ownerRenderer is always laid out before the RenderSVGRoot and not the other way round. This is the source of current flakiness bugs.

In trunk FrameView contains several special hacks, to layout the ownerRenderers view, after the RenderSVGRoots view finished layout. This worked without flakiness as RenderSVGRoot used to directly call computeReplacedLogicalWidth/Height on the ownerRenderer, which is now gone.
Fortunately we can keep the new design, and can remove all hacks out of RenderSVGRoot/FrameView, if we can guarantee that the ownerRenderer FrameView is laid out before the RenderSVGRoot FrameView. I have a patch doing this, and it works nicely, finally removing the FrameView hacks, that had to be invented before, to make the size negotiation work within the old design.
Comment 1 Nikolas Zimmermann 2012-01-26 06:41:10 PST
Created attachment 124115 [details]
Patch
Comment 2 Nikolas Zimmermann 2012-01-26 06:49:11 PST
*** Bug 66320 has been marked as a duplicate of this bug. ***
Comment 3 Nikolas Zimmermann 2012-01-26 06:50:37 PST
*** Bug 77061 has been marked as a duplicate of this bug. ***
Comment 4 Andreas Kling 2012-01-26 06:51:37 PST
Comment on attachment 124115 [details]
Patch

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

r=me

> Source/WebCore/page/FrameView.cpp:912
> +    // If the embedded SVG document appears the first time, the ownerRenderer has releady finished

Typo, already.

> Source/WebCore/page/FrameView.cpp:913
> +    // layout without knowing about the existance of the embedded SVG document, because RenderReplaced

Typo, existence.

> LayoutTests/ChangeLog:8
> +        Introduce a testcase that fails reproducable w/o needed guard malloc, if we ever regress <object> sizing again.

Typo, reproducibly.
Comment 5 Nikolas Zimmermann 2012-01-26 07:05:23 PST
Committed r106001: <http://trac.webkit.org/changeset/106001>
Comment 6 Nikolas Zimmermann 2012-01-26 07:06:27 PST
Comment on attachment 124115 [details]
Patch

Thanks for the quick review! Fixed your comments.