Chrome/Mac layout error on Google Docs Demo page

Chrome/Mac layout error on Google Docs Demo page (https://docs.google.com/demo)

Avoid collapsing the preferred size reported by RenderView when WebKit
is unable to calculate scrollable height.

BUG=103423
TEST=http://docs.google.com/demo 'Sign In' and 'Get Started' shouldn't wrap.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112959

[davidbarr, 2011-11-30 00:21:36]

My understanding was that tests are required for chromium patches.
I have a test case that fails manual testing without the patch in
canary but always passes when executed by interactive_ui_tests.

[levin, 2011-12-01 00:12:55]

On 2011/11/30 00:21:36, davidbarr wrote:
> My understanding was that tests are required for chromium patches.
> I have a test case that fails manual testing without the patch in
> canary but always passes when executed by interactive_ui_tests.

If it is too hard, that is ok as I don't think other folks have tested this and
maybe that is why.

The bigger issue is why is this needed? When is the value 0? Why does this cause
an issue in a web page? etc.

[davidbarr, 2011-12-01 04:24:34]

On 2011/12/01 00:12:55, levin wrote:
> On 2011/11/30 00:21:36, davidbarr wrote:
> > My understanding was that tests are required for chromium patches.
> > I have a test case that fails manual testing without the patch in
> > canary but always passes when executed by interactive_ui_tests.
> 
> If it is too hard, that is ok as I don't think other folks have tested this
and
> maybe that is why.
> 
> The bigger issue is why is this needed? When is the value 0? Why does this
cause
> an issue in a web page? etc.

WebKit reports a height of zero when it has insufficient information to compute
the height. As best I can tell, this only happens during the initial layout. In
this case, as we the preferred size events are coalesced, it seems sensible to
leave the height as it was. When the layout completes an accurate preferred
height will be received.

As to how this causes the layout bug, the only clue I've found is that height
calculation forces a layout without external styles and that may in turn cause a
pathological layout that WebKit fails to update when external stylesheets are
available.

[levin, 2011-12-01 09:56:31]

On 2011/12/01 04:24:34, davidbarr wrote:
> On 2011/12/01 00:12:55, levin wrote:
> > On 2011/11/30 00:21:36, davidbarr wrote:
> > > My understanding was that tests are required for chromium patches.
> > > I have a test case that fails manual testing without the patch in
> > > canary but always passes when executed by interactive_ui_tests.
> > 
> > If it is too hard, that is ok as I don't think other folks have tested this
> and
> > maybe that is why.
> > 
> > The bigger issue is why is this needed? When is the value 0? Why does this
> cause
> > an issue in a web page? etc.
> 
> WebKit reports a height of zero when it has insufficient information to
compute
> the height. As best I can tell, this only happens during the initial layout.
In
> this case, as we the preferred size events are coalesced, it seems sensible to
> leave the height as it was. When the layout completes an accurate preferred
> height will be received.

Maybe we should skip sending a preferred size in this case? (But you may not be
able to tell that at this level. Let me sleep on it.... I should reply again by
your morning. If not, ping me.)




> 
> As to how this causes the layout bug, the only clue I've found is that height
> calculation forces a layout without external styles and that may in turn cause
a
> pathological layout that WebKit fails to update when external stylesheets are
> available.

[levin, 2011-12-04 23:28:56]

lgtm

[commit-bot: I haz the power, 2011-12-05 03:21:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidbarr@chromium.org/8670014/1

[commit-bot: I haz the power, 2011-12-05 03:21:08]

Presubmit check for 8670014-1 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for: content/renderer/render_view_impl.cc

[jam, 2011-12-05 04:23:24]

rubberstamp lgtm

[commit-bot: I haz the power, 2011-12-05 04:25:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidbarr@chromium.org/8670014/1

[commit-bot: I haz the power, 2011-12-05 08:23:02]

Change committed as 112959