Inherit 'viewBox' into view spec if it is valid

Inherit 'viewBox' into view spec if it is valid

When constructing a view spec from a fragment identifier, the
(negated) hasEmptyViewBox() condition was used to determine if the
viewBox value should be set from the element. This condition however
will equate to:

  !(is-valid && is-empty) => !is-valid || !is-empty

meaning that invalid viewBox values would be transferred. In the case
where a <view> element without a 'viewBox' specified was referenced, the
'viewBox' value from the root could be overwritten by an invalid one.

Use only the validity of the 'viewBox' to determine if the value should
be inherited.

BUG=702380
Review-Url: https://codereview.chromium.org/2753773009
Cr-Commit-Position: refs/heads/master@{#457921}
Committed: https://chromium.googlesource.com/chromium/src/+/d2c24342da278481fc6041e235f8690828e6760d

[fs, 2017-03-17 15:07:16]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 15:07:38]

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

[fs, 2017-03-17 16:00:14]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 16:00:39]

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, 2017-03-17 16:18:10]

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

[commit-bot: I haz the power, 2017-03-17 16:18:11]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fs, 2017-03-18 00:32:18]

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

[pdr., 2017-03-18 01:27:30]

LGTM

SVGSVGElement::localCoordinateSpaceTransform's use also looks a bit suspect.

[fs, 2017-03-18 01:31:47]

On 2017/03/18 at 01:27:30, pdr wrote:
> LGTM
> 
> SVGSVGElement::localCoordinateSpaceTransform's use also looks a bit suspect.

Yes... I need (to remember) to make a TC (and sleep.)

[fs, 2017-03-18 01:32:05]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2017-03-18 01:32:56]

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, 2017-03-18 01:46:37]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489800725440790,
"parent_rev": "3685db28900ee3e99c5e2e6ce8bfecb78805f1de", "commit_rev":
"d2c24342da278481fc6041e235f8690828e6760d"}

[commit-bot: I haz the power, 2017-03-18 01:47:44]

Description was changed from

==========
Inherit 'viewBox' into view spec if it is valid

When constructing a view spec from a fragment identifier, the
(negated) hasEmptyViewBox() condition was used to determine if the
viewBox value should be set from the element. This condition however
will equate to:

  !(is-valid && is-empty) => !is-valid || !is-empty

meaning that invalid viewBox values would be transferred. In the case
where a <view> element without a 'viewBox' specified was referenced, the
'viewBox' value from the root could be overwritten by an invalid one.

Use only the validity of the 'viewBox' to determine if the value should
be inherited.

BUG=702380
==========

to

==========
Inherit 'viewBox' into view spec if it is valid

When constructing a view spec from a fragment identifier, the
(negated) hasEmptyViewBox() condition was used to determine if the
viewBox value should be set from the element. This condition however
will equate to:

  !(is-valid && is-empty) => !is-valid || !is-empty

meaning that invalid viewBox values would be transferred. In the case
where a <view> element without a 'viewBox' specified was referenced, the
'viewBox' value from the root could be overwritten by an invalid one.

Use only the validity of the 'viewBox' to determine if the value should
be inherited.

BUG=702380

Review-Url: https://codereview.chromium.org/2753773009
Cr-Commit-Position: refs/heads/master@{#457921}
Committed:
https://chromium.googlesource.com/chromium/src/+/d2c24342da278481fc6041e235f8...
==========

[commit-bot: I haz the power, 2017-03-18 01:47:45]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/d2c24342da278481fc6041e235f8...

[fs, 2017-03-20 08:54:42]

On 2017/03/18 at 01:31:47, fs wrote:
> On 2017/03/18 at 01:27:30, pdr wrote:
> > LGTM
> > 
> > SVGSVGElement::localCoordinateSpaceTransform's use also looks a bit suspect.
> 
> Yes... I need (to remember) to make a TC (and sleep.)

Filed crbug.com/703050 for some followup.