Don't ignore size updates that have a width and height == 0.

Don't ignore size updates that have a width and height == 0.

It is perfectly legal for the Android WebView to have a zero height,
this is frequenly how a new WebView is first laid out. Rejecting the
initial (width, 0) size update causes the initial layout to be
too narrow.

BUG=344885
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167543

[mkosiba (inactive), 2014-02-19 15:06:21]

PTAL

[mnaganov (inactive), 2014-02-19 16:13:50]

On 2014/02/19 15:06:21, mkosiba wrote:
> PTAL

Sorry, I don't see deltas, so can't add proper comments.

In general this looks valid, but I'm concerned that it's worth adding 'isZero'
method to WebSize. First, the name isn't very descriptive (I can't clearly tell
the difference between 'isEmpty' vs. 'isZero' w/o looking at their sources),
second, it's only used in one place.

[mkosiba (inactive), 2014-02-19 16:20:50]

On 2014/02/19 16:13:50, Mikhail Naganov (Cr) wrote:
> On 2014/02/19 15:06:21, mkosiba wrote:
> > PTAL
> 
> Sorry, I don't see deltas, so can't add proper comments.

ah.. rietveld acting up. I re-uploaded the CL, should be fine now.

> In general this looks valid, but I'm concerned that it's worth adding 'isZero'
> method to WebSize. First, the name isn't very descriptive (I can't clearly
tell
> the difference between 'isEmpty' vs. 'isZero' w/o looking at their sources),
> second, it's only used in one place.

The reason I added the method is because many webkit "size" types already have
it
https://code.google.com/p/chromium/codesearch#search/&q=isZero%20package:%5Ec...
I essentially copied the semantics from IntSize.

[mnaganov (inactive), 2014-02-19 16:26:10]

On 2014/02/19 16:20:50, mkosiba wrote:
> On 2014/02/19 16:13:50, Mikhail Naganov (Cr) wrote:
> > On 2014/02/19 15:06:21, mkosiba wrote:
> > > PTAL
> > 
> > Sorry, I don't see deltas, so can't add proper comments.
> 
> ah.. rietveld acting up. I re-uploaded the CL, should be fine now.
> 
> > In general this looks valid, but I'm concerned that it's worth adding
'isZero'
> > method to WebSize. First, the name isn't very descriptive (I can't clearly
> tell
> > the difference between 'isEmpty' vs. 'isZero' w/o looking at their sources),
> > second, it's only used in one place.
> 
> The reason I added the method is because many webkit "size" types already have
> it
>
https://code.google.com/p/chromium/codesearch#search/&q=isZero%2520package:%2...
> I essentially copied the semantics from IntSize.

Right, so the name is correct then. But their implementations are a bit
different -- they check for !width && !height. This difference can lead to
subtle errors later...

[mkosiba (inactive), 2014-02-19 16:49:21]

On 2014/02/19 16:26:10, Mikhail Naganov (Cr) wrote:
> On 2014/02/19 16:20:50, mkosiba wrote:
> > On 2014/02/19 16:13:50, Mikhail Naganov (Cr) wrote:
> > > On 2014/02/19 15:06:21, mkosiba wrote:
> > > > PTAL
> > > 
> > > Sorry, I don't see deltas, so can't add proper comments.
> > 
> > ah.. rietveld acting up. I re-uploaded the CL, should be fine now.
> > 
> > > In general this looks valid, but I'm concerned that it's worth adding
> 'isZero'
> > > method to WebSize. First, the name isn't very descriptive (I can't clearly
> > tell
> > > the difference between 'isEmpty' vs. 'isZero' w/o looking at their
sources),
> > > second, it's only used in one place.
> > 
> > The reason I added the method is because many webkit "size" types already
have
> > it
> >
>
https://code.google.com/p/chromium/codesearch#search/&q=isZero%252520package:...
> > I essentially copied the semantics from IntSize.
> 
> Right, so the name is correct then. But their implementations are a bit
> different -- they check for !width && !height. This difference can lead to
> subtle errors later...

oh, sorry, my bad - I intended for this to be !width && !height

[aelias_OOO_until_Jul13, 2014-02-19 19:18:59]

WebSize is meant primary as a transport type between Blink and Chromium repos
rather than something to perform computations on so we try to not add methods to
it.  Probably m_size should just be changed to be an IntSize to make this kind
of thing easier... failing that, please just write the logic directly in the
method.

[mkosiba (inactive), 2014-02-20 13:53:24]

thanks Alex! I removed the isZero method. Changing WebSize->IntSize made this
change bigger than it needs to be and changing the check didn't look that bad.

[mnaganov (inactive), 2014-02-20 15:50:25]

Looks good modulo comment.

https://codereview.chromium.org/172203003/diff/180001/Source/web/tests/WebFra...
File Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/172203003/diff/180001/Source/web/tests/WebFra...
Source/web/tests/WebFrameTest.cpp:873:
webViewHelper.webView()->settings()->setUseWideViewport(true);
I think this isn't needed, as wide viewport is turned on by default. Even having
it off will not do anything unless you enable WideViewportQuirk.

[mkosiba (inactive), 2014-02-20 15:59:32]

thanks Mikhail, too much copy-paste.

https://codereview.chromium.org/172203003/diff/180001/Source/web/tests/WebFra...
File Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/172203003/diff/180001/Source/web/tests/WebFra...
Source/web/tests/WebFrameTest.cpp:873:
webViewHelper.webView()->settings()->setUseWideViewport(true);
On 2014/02/20 15:50:25, Mikhail Naganov (Cr) wrote:
> I think this isn't needed, as wide viewport is turned on by default. Even
having
> it off will not do anything unless you enable WideViewportQuirk.

Done.

[aelias_OOO_until_Jul13, 2014-02-20 18:15:47]

lgtm

[mkosiba (inactive), 2014-02-20 18:55:16]

The CQ bit was checked by mkosiba@chromium.org

[commit-bot: I haz the power, 2014-02-20 18:55:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/172203003/230001

[commit-bot: I haz the power, 2014-02-20 22:26:49]

Change committed as 167543