Avoid crashing if multiple target popup is empty.

Avoid crashing if multiple target popup is empty.

If the bitmap wasn't allocated, return false instead.

BUG=411505
Committed: https://crrev.com/0ed0918111d020972e00feda36ae0b68075d6d0f
Cr-Commit-Position: refs/heads/master@{#314009}

[jbauman, 2015-01-28 01:05:58]

jbauman@chromium.org changed reviewers:
+ sievers@chromium.org

[no sievers, 2015-01-28 18:26:23]

https://codereview.chromium.org/879143002/diff/1/content/renderer/render_view...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/879143002/diff/1/content/renderer/render_view...
content/renderer/render_view_impl.cc:3942: if (!new_total_scale)
So in case that canvas_size being 0 is the problem, could we already catch that
here? Or is it unexpected and should be DCHECK'd?

https://codereview.chromium.org/879143002/diff/1/content/renderer/render_view...
content/renderer/render_view_impl.cc:3957: if (!shared_bitmap) {
nit: consider if (shared_bitmap) {line 3961-3996} instead

[jbauman, 2015-01-30 01:12:05]

On 2015/01/28 18:26:23, sievers wrote:
>
https://codereview.chromium.org/879143002/diff/1/content/renderer/render_view...
> File content/renderer/render_view_impl.cc (right):
> 
>
https://codereview.chromium.org/879143002/diff/1/content/renderer/render_view...
> content/renderer/render_view_impl.cc:3942: if (!new_total_scale)
> So in case that canvas_size being 0 is the problem, could we already catch
that
> here? Or is it unexpected and should be DCHECK'd?

This also catches the case where the size is really huge and the bitmap size
would overflow. None of these should really be happening, but they shouldn't
crash.
> 
>
https://codereview.chromium.org/879143002/diff/1/content/renderer/render_view...
> content/renderer/render_view_impl.cc:3957: if (!shared_bitmap) {
> nit: consider if (shared_bitmap) {line 3961-3996} instead

[no sievers, 2015-01-30 02:09:29]

Do you think we could somehow find out more, even if it's CHECKs()?

We don't usually handle allocation failures (but just crash).

So if there is a bug in our code that causes us to come up with a 0 size, or
with a size way too bug by accident, that would be good to know. Rather than
silently breaking the feature.

https://codereview.chromium.org/879143002/diff/1/content/renderer/render_view...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/879143002/diff/1/content/renderer/render_view...
content/renderer/render_view_impl.cc:3957: if (!shared_bitmap) {
On 2015/01/28 18:26:23, sievers wrote:
> nit: consider if (shared_bitmap) {line 3961-3996} instead

and maybe this nit

[jbauman, 2015-01-30 03:02:18]

On 2015/01/30 02:09:29, sievers wrote:
> Do you think we could somehow find out more, even if it's CHECKs()?
> 
> We don't usually handle allocation failures (but just crash).
> 
> So if there is a bug in our code that causes us to come up with a 0 size, or
> with a size way too bug by accident, that would be good to know. Rather than
> silently breaking the feature.

Ok, changed the code to handle the 0-size case somewhat differently, and added a
CHECK if the size is too large.
> 
>
https://codereview.chromium.org/879143002/diff/1/content/renderer/render_view...
> File content/renderer/render_view_impl.cc (right):
> 
>
https://codereview.chromium.org/879143002/diff/1/content/renderer/render_view...
> content/renderer/render_view_impl.cc:3957: if (!shared_bitmap) {
> On 2015/01/28 18:26:23, sievers wrote:
> > nit: consider if (shared_bitmap) {line 3961-3996} instead
> 
> and maybe this nit

[no sievers, 2015-01-30 17:38:37]

lgtm

[jbauman, 2015-01-30 22:29:32]

The CQ bit was checked by jbauman@chromium.org

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

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

[commit-bot: I haz the power, 2015-01-30 22:34:17]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-30 22:35:19]

Patchset 2 (id:??) landed as
https://crrev.com/0ed0918111d020972e00feda36ae0b68075d6d0f
Cr-Commit-Position: refs/heads/master@{#314009}