Bug 107990 – Add asserts about when you can call computePreferredLogicalWidths

• Status: RESOLVED
• Resolution: WONTFIX
• Component: New Bugs

[Ojan Vafai]

Add asserts about when you can call computePreferredLogicalWidths

[Ojan Vafai]

Created attachment 184828 [details]
Patch

[Ojan Vafai]

I started out with an assert that we only call computePreferredLogicalWidths from within layout. It turns out there are a bunch of cases where we legitimately call computePreferredLogicalWidths outside of layout. So, I ended up with this. This should maybe just be merged into the patch on bug 104829. It's a little weird to commit by itself.

[Ojan Vafai]

Created attachment 186941 [details]
Patch

[Elliott Sprehn]

Comment on attachment 186941 [details]
Patch

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

Seems strange to do this and not ifdef everything out for release builds.

> Source/WebCore/rendering/RenderBox.cpp:889
> +        assertIsReadyForWidthComputation();

Should be inside the ifdef above it.

> Source/WebCore/rendering/RenderView.cpp:181
> +    TemporaryChange<bool> changeInAutoSize(const_cast<bool&>(m_canComputeIntrinsicWidths), true);

Is this just overhead on the method calls in release builds? It feels like we should ifdef all the code related to m_canComputeIntrinsicWidths out unless there's some reason for it to exist in release builds.

> Source/WebCore/rendering/RenderView.h:237
> +    virtual LayoutUnit maxPreferredLogicalWidth() const;

Missing OVERRIDE on both of these.

[Ojan Vafai]

I'm not convinced this assert adds much value. I'm on the fence, so if Hyatt or someone likes this, I can still add it.