Skip hit testing containers with invalid bounding boxes

Skip hit testing containers with invalid bounding boxes

When an SVG container has no children, it's bounding box is considered
invalid. We had an assert that would fire when hit testing invalid
containers, but this assert should just be an if statement because it
is okay to hit test empty containers.

BUG=629274
Committed: https://crrev.com/0bb88dbb01568d8f386365abac2bfd148bb9b2b6
Cr-Commit-Position: refs/heads/master@{#411536}

[pdr., 2016-08-10 23:23:15]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[pdr., 2016-08-10 23:23:25]

pdr@chromium.org changed reviewers:
+ fs@opera.com, schenney@chromium.org

[commit-bot: I haz the power, 2016-08-10 23:24:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 01:35:02]

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

[commit-bot: I haz the power, 2016-08-11 01:35:03]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[fs, 2016-08-11 15:22:04]

lgtm

https://codereview.chromium.org/2234173002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/svg/hittest/empty-container.html (right):

https://codereview.chromium.org/2234173002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/hittest/empty-container.html:15: Hit testing
an empty container should not crash.
To mesh better with testharness, this should probably be put as a <title> (which
then will be picked up as the "name" for the test().)

https://codereview.chromium.org/2234173002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.cpp (right):

https://codereview.chromium.org/2234173002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.cpp:192: // Check
for a valid bounding box because it can be invalid for empty containers.
Nit: s/can/will/ ?

[pdr., 2016-08-11 22:51:24]

https://codereview.chromium.org/2234173002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/svg/hittest/empty-container.html (right):

https://codereview.chromium.org/2234173002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/hittest/empty-container.html:15: Hit testing
an empty container should not crash.
On 2016/08/11 at 15:22:04, fs (OoO until Aug 15) wrote:
> To mesh better with testharness, this should probably be put as a <title>
(which then will be picked up as the "name" for the test().)

Done and confirmed the failure text looks nicer now.

https://codereview.chromium.org/2234173002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.cpp (right):

https://codereview.chromium.org/2234173002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.cpp:192: // Check
for a valid bounding box because it can be invalid for empty containers.
On 2016/08/11 at 15:22:04, fs (OoO until Aug 15) wrote:
> Nit: s/can/will/ ?

weaselwords!

Fixed

[pdr., 2016-08-11 22:51:26]

The CQ bit was checked by pdr@chromium.org

[pdr., 2016-08-11 22:51:26]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/2234173002/#ps20001
(title: "Address reviewer comments")

[commit-bot: I haz the power, 2016-08-11 22:52:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-12 00:15:17]

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

[commit-bot: I haz the power, 2016-08-12 00:15:18]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pdr., 2016-08-12 00:16:02]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2016-08-12 00:17:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-12 03:17:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-12 03:20:32]

Description was changed from

==========
Skip hit testing containers with invalid bounding boxes

When an SVG container has no children, it's bounding box is considered
invalid. We had an assert that would fire when hit testing invalid
containers, but this assert should just be an if statement because it
is okay to hit test empty containers.

BUG=629274
==========

to

==========
Skip hit testing containers with invalid bounding boxes

When an SVG container has no children, it's bounding box is considered
invalid. We had an assert that would fire when hit testing invalid
containers, but this assert should just be an if statement because it
is okay to hit test empty containers.

BUG=629274

Committed: https://crrev.com/0bb88dbb01568d8f386365abac2bfd148bb9b2b6
Cr-Commit-Position: refs/heads/master@{#411536}
==========

[commit-bot: I haz the power, 2016-08-12 03:20:33]

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