views: Early out of painting Views with empty bounds.

views: Early out of painting Views with empty bounds.

They are empty so they aren't visible. Trying to paint a view with
empty bounds actually leads to DCHECK fails when they try to use the
empty size incorrectly. With slimming paint this could happen for the
first frame when we paint /everything/ to try cache it all until
invalidation. Instead, defer painting empty things until they actually
have something to paint. This also means less wasted time painting
subtrees that aren't visible.

R=sky@chromium.org, weiliangc@chromium.org, sky, weiliangc
BUG=498066
Committed: https://chromium.googlesource.com/chromium/src/+/980b6840952ecfd1d23dd30e536c020d5d9641be

[weiliangc, 2015-06-09 22:04:42]

LGTM

https://codereview.chromium.org/1166373008/diff/1/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/1166373008/diff/1/ui/views/view.cc#newcode740
ui/views/view.cc:740: if (bounds().IsEmpty())
nit: We can use size() instead?

[danakj, 2015-06-09 22:05:15]

https://codereview.chromium.org/1166373008/diff/1/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/1166373008/diff/1/ui/views/view.cc#newcode740
ui/views/view.cc:740: if (bounds().IsEmpty())
On 2015/06/09 22:04:42, weiliangc wrote:
> nit: We can use size() instead?

Ya, though size() is bounds().size() so that's an extra indirection which is how
I ended up here.

[weiliangc, 2015-06-09 22:18:19]

On 2015/06/09 at 22:05:15, danakj wrote:
> https://codereview.chromium.org/1166373008/diff/1/ui/views/view.cc
> File ui/views/view.cc (right):
> 
> https://codereview.chromium.org/1166373008/diff/1/ui/views/view.cc#newcode740
> ui/views/view.cc:740: if (bounds().IsEmpty())
> On 2015/06/09 22:04:42, weiliangc wrote:
> > nit: We can use size() instead?
> 
> Ya, though size() is bounds().size() so that's an extra indirection which is
how I ended up here.

Either way is fine. I don't think it matters since gfx::Rect::IsEmpty() is just
size().IsEmpty()...

[sky, 2015-06-09 22:25:07]

LGTM

[danakj, 2015-06-09 22:27:52]

On Tue, Jun 9, 2015 at 3:18 PM, <weiliangc@chromium.org> wrote:

> On 2015/06/09 at 22:05:15, danakj wrote:
>
>> https://codereview.chromium.org/1166373008/diff/1/ui/views/view.cc
>> File ui/views/view.cc (right):
>>
>
>
>>
https://codereview.chromium.org/1166373008/diff/1/ui/views/view.cc#newcode740
>> ui/views/view.cc:740: if (bounds().IsEmpty())
>> On 2015/06/09 22:04:42, weiliangc wrote:
>> > nit: We can use size() instead?
>>
>
>  Ya, though size() is bounds().size() so that's an extra indirection which
>> is
>>
> how I ended up here.
>
> Either way is fine. I don't think it matters since gfx::Rect::IsEmpty() is
> just
> size().IsEmpty()...
>

Aha.. ya..


>
> https://codereview.chromium.org/1166373008/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2015-06-09 22:29:13]

The CQ bit was checked by danakj@chromium.org

[danakj, 2015-06-09 22:29:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
weiliangc@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1166373008/#ps20001
(title: "emptyview: size")

[commit-bot: I haz the power, 2015-06-09 22:30:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1166373008/20001

[commit-bot: I haz the power, 2015-06-10 00:31:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-10 00:31:31]

Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[danakj, 2015-06-10 00:32:15]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2015-06-10 00:37:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1166373008/20001

[danakj, 2015-06-10 01:01:38]

Committed patchset #2 (id:20001) manually as
980b6840952ecfd1d23dd30e536c020d5d9641be (presubmit successful).