Replace ASSERT with DCHECK in core/svg/

Replace ASSERT with DCHECK in core/svg/

BUG=697687
Review-Url: https://codereview.chromium.org/2738863002
Cr-Commit-Position: refs/heads/master@{#456946}
Committed: https://chromium.googlesource.com/chromium/src/+/24cf3269c241270f97890a977a3989577b2c4d31

[mrunal, 2017-03-08 03:20:45]

Description was changed from

==========
Replace ASSERT with DCHECK in core/svg/

BUG=697687
==========

to

==========
Replace ASSERT with DCHECK in core/svg/

BUG=697687
==========

[mrunal, 2017-03-08 03:20:45]

mrunal.kapade@intel.com changed reviewers:
+ fs@opera.com, tkent@chromium.org

[mrunal, 2017-03-08 03:21:02]

The CQ bit was checked by mrunal.kapade@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 03:21:38]

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

[mrunal, 2017-03-08 03:22:22]

The CQ bit was unchecked by mrunal.kapade@intel.com

[mrunal, 2017-03-09 00:30:48]

The CQ bit was checked by mrunal.kapade@intel.com to run a CQ dry run

[mrunal, 2017-03-09 00:30:58]

The CQ bit was unchecked by mrunal.kapade@intel.com

[mrunal, 2017-03-09 00:34:16]

The CQ bit was checked by mrunal.kapade@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-09 00:34:42]

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-09 00:41:55]

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

[commit-bot: I haz the power, 2017-03-09 00:41:57]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mrunal, 2017-03-14 21:34:04]

fs, can you PTAL?

[fs, 2017-03-14 21:50:23]

LGTM w/ nits

https://codereview.chromium.org/2738863002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGAnimationElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGAnimationElement.cpp:333:
DCHECK_GE(valuesCount, static_cast<unsigned>(1));
Nit: 1u

https://codereview.chromium.org/2738863002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGAnimationElement.cpp:400:
DCHECK_GT(m_keyTimes.size(), static_cast<unsigned>(1));
Nit: 1u

https://codereview.chromium.org/2738863002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGAnimationElement.cpp:455:
DCHECK_GE(valuesCount, static_cast<unsigned>(1));
1u

https://codereview.chromium.org/2738863002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGTransformList.cpp (right):

https://codereview.chromium.org/2738863002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGTransformList.cpp:441: DCHECK_EQ(length(),
static_cast<unsigned>(1));
1u

https://codereview.chromium.org/2738863002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGTransformList.cpp:522: DCHECK_EQ(length(),
static_cast<unsigned>(1));
1u

[mrunal, 2017-03-14 22:14:52]

The CQ bit was checked by mrunal.kapade@intel.com

[mrunal, 2017-03-14 22:14:53]

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/2738863002/#ps100001
(title: "static_cast<unsigned>(1) > 1u")

[commit-bot: I haz the power, 2017-03-14 22:15:36]

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

[mrunal, 2017-03-14 22:30:57]

The CQ bit was unchecked by mrunal.kapade@intel.com

[mrunal, 2017-03-14 22:31:13]

The CQ bit was checked by mrunal.kapade@intel.com

[commit-bot: I haz the power, 2017-03-14 22:32:05]

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

[tkent, 2017-03-14 22:33:14]

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGFEBlendElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGFEBlendElement.cpp:155: DCHECK(input1 &&
input2);
Split it to two DCHECKs.
  DCHECK(input1);
  DCHECK(input2);

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGFECompositeElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGFECompositeElement.cpp:135: DCHECK(input1
&& input2);
Split it to two DCHECKs.
  DCHECK(input1);
  DCHECK(input2);

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGFEDisplacementMapElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGFEDisplacementMapElement.cpp:118:
DCHECK(input1 && input2);
Split it to two DHCECKs.
  DCHECK(input1);
  DCHECK(input2);

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGPathStringBuilder.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGPathStringBuilder.cpp:77:
DCHECK(segment.command > PathSegUnknown &&
Split it to two DCHECKs.
  DCHECK_GT(segment.command, PathSegUnknown);
  DCHECK_LE(segment.command, PathSegCurveToQuadraticSmoothRel);

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGPolyElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGPolyElement.cpp:51: DCHECK(it != itEnd);
Use DCHECK_NE if possible.

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGUseElement.cpp:357:
DCHECK(!(targetIterator != targetRange.end()));
Use DCHECK_EQ if possible.

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGUseElement.cpp:683:
DCHECK(isStructurallyExternal() && m_haveFiredLoadEvent);
Split it to two DCHECKs
  DCHECK((isStructurallyExternal());
  DCHECK(m_haveFiredLoadEvent);

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.cpp:109:
DCHECK(itAnimation != scheduled->end());
Use DCHECK_NE if possible

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp:758:
DCHECK(currentTime == minimumTime);
Use DCHECK_EQ if possible

[mrunal, 2017-03-14 23:08:49]

The CQ bit was unchecked by mrunal.kapade@intel.com

[mrunal, 2017-03-14 23:52:14]

Sorry tkent, I jumped the gun on committing the patch. I have made suggested
changes but some encounter compiler error.

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGFEBlendElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGFEBlendElement.cpp:155: DCHECK(input1 &&
input2);
On 2017/03/14 22:33:14, tkent wrote:
> Split it to two DCHECKs.
>   DCHECK(input1);
>   DCHECK(input2);

Done.

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGFECompositeElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGFECompositeElement.cpp:135: DCHECK(input1
&& input2);
On 2017/03/14 22:33:14, tkent wrote:
> Split it to two DCHECKs.
>   DCHECK(input1);
>   DCHECK(input2);

Done.

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGFEDisplacementMapElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGFEDisplacementMapElement.cpp:118:
DCHECK(input1 && input2);
On 2017/03/14 22:33:14, tkent wrote:
> Split it to two DHCECKs.
>   DCHECK(input1);
>   DCHECK(input2);

Done.

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGPathStringBuilder.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGPathStringBuilder.cpp:77:
DCHECK(segment.command > PathSegUnknown &&
On 2017/03/14 22:33:14, tkent wrote:
> Split it to two DCHECKs.
>   DCHECK_GT(segment.command, PathSegUnknown);
>   DCHECK_LE(segment.command, PathSegCurveToQuadraticSmoothRel);

Done.

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGPolyElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGPolyElement.cpp:51: DCHECK(it != itEnd);
On 2017/03/14 22:33:14, tkent wrote:
> Use DCHECK_NE if possible.

It throws an error,
../../base/logging.h:659:3: error: no matching function for call to
'MakeCheckOpValueString'
  MakeCheckOpValueString(&ss, v1);
  ^~~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:704:1: note: in instantiation of function template
specialization
'logging::MakeCheckOpString<blink::SVGListPropertyHelper<blink::SVGPointList,
blink::SVGPoint>::ConstIterator,
blink::SVGListPropertyHelper<blink::SVGPointList,
blink::SVGPoint>::ConstIterator>' requested here
DEFINE_CHECK_OP_IMPL(NE, !=)
^
../../base/logging.h:695:25: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
      return ::logging::MakeCheckOpString(v1, v2, names);                    \
                        ^
../../third_party/WebKit/Source/core/svg/SVGPolyElement.cpp:51:3: note: in
instantiation of function template specialization
'logging::CheckNEImpl<blink::SVGListPropertyHelper<blink::SVGPointList,
blink::SVGPoint>::ConstIterator,
blink::SVGListPropertyHelper<blink::SVGPointList,
blink::SVGPoint>::ConstIterator>' requested here
  DCHECK_NE(it, itEnd);
  ^
../../base/logging.h:900:31: note: expanded from macro 'DCHECK_NE'
#define DCHECK_NE(val1, val2) DCHECK_OP(NE, !=, val1, val2)
                              ^
../../base/logging.h:853:18: note: expanded from macro 'DCHECK_OP'
      ::logging::Check##name##Impl((val1), (val2),                     \
                 ^

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGUseElement.cpp:357:
DCHECK(!(targetIterator != targetRange.end()));
On 2017/03/14 22:33:14, tkent wrote:
> Use DCHECK_EQ if possible.

It throws an error,
../../base/logging.h:703:26: error: invalid operands to binary expression
('const
blink::TraversalInclusiveDescendantIterator<blink::Traversal<blink::SVGElement>
>' and 'const
blink::TraversalInclusiveDescendantIterator<blink::Traversal<blink::SVGElement>
>')
DEFINE_CHECK_OP_IMPL(EQ, ==)
~~~~~~~~~~~~~~~~~~~~~~~~~^~~
../../base/logging.h:692:12: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
    if (v1 op v2)                                                            \
        ~~ ^  ~~
../../third_party/WebKit/Source/core/svg/SVGUseElement.cpp:357:3: note: in
instantiation of function template specialization
'logging::CheckEQImpl<blink::TraversalInclusiveDescendantIterator<blink::Traversal<blink::SVGElement>
>,
blink::TraversalInclusiveDescendantIterator<blink::Traversal<blink::SVGElement>
> >' requested here
  DCHECK_EQ(targetIterator, targetRange.end());
  ^
../../base/logging.h:899:31: note: expanded from macro 'DCHECK_EQ'
#define DCHECK_EQ(val1, val2) DCHECK_OP(EQ, ==, val1, val2)
                              ^
../../base/logging.h:853:18: note: expanded from macro 'DCHECK_OP'
      ::logging::Check##name##Impl((val1), (val2),                     \
                 ^

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGUseElement.cpp:683:
DCHECK(isStructurallyExternal() && m_haveFiredLoadEvent);
On 2017/03/14 22:33:14, tkent wrote:
> Split it to two DCHECKs
>   DCHECK((isStructurallyExternal());
>   DCHECK(m_haveFiredLoadEvent);

Done.

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.cpp:109:
DCHECK(itAnimation != scheduled->end());
On 2017/03/14 22:33:14, tkent wrote:
> Use DCHECK_NE if possible

It throws a compiler error,
../../base/logging.h:659:3: error: no matching function for call to
'MakeCheckOpValueString'
  MakeCheckOpValueString(&ss, v1);
  ^~~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:704:1: note: in instantiation of function template
specialization
'logging::MakeCheckOpString<WTF::LinkedHashSetIterator<WTF::LinkedHashSet<blink::WeakMember<blink::SVGSMILElement>,
WTF::MemberHash<blink::SVGSMILElement>,
WTF::HashTraits<blink::WeakMember<blink::SVGSMILElement> >,
blink::HeapAllocator> >,
WTF::LinkedHashSetIterator<WTF::LinkedHashSet<blink::WeakMember<blink::SVGSMILElement>,
WTF::MemberHash<blink::SVGSMILElement>,
WTF::HashTraits<blink::WeakMember<blink::SVGSMILElement> >,
blink::HeapAllocator> > >' requested here
DEFINE_CHECK_OP_IMPL(NE, !=)
^
../../base/logging.h:695:25: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
      return ::logging::MakeCheckOpString(v1, v2, names);                    \
                        ^
../../third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.cpp:109:3:
note: in instantiation of function template specialization
'logging::CheckNEImpl<WTF::LinkedHashSetIterator<WTF::LinkedHashSet<blink::WeakMember<blink::SVGSMILElement>,
WTF::MemberHash<blink::SVGSMILElement>,
WTF::HashTraits<blink::WeakMember<blink::SVGSMILElement> >,
blink::HeapAllocator> >,
WTF::LinkedHashSetIterator<WTF::LinkedHashSet<blink::WeakMember<blink::SVGSMILElement>,
WTF::MemberHash<blink::SVGSMILElement>,
WTF::HashTraits<blink::WeakMember<blink::SVGSMILElement> >,
blink::HeapAllocator> > >' requested here
  DCHECK_NE(itAnimation, scheduled->end());
  ^
../../base/logging.h:900:31: note: expanded from macro 'DCHECK_NE'
#define DCHECK_NE(val1, val2) DCHECK_OP(NE, !=, val1, val2)
                              ^
../../base/logging.h:853:18: note: expanded from macro 'DCHECK_OP'
      ::logging::Check##name##Impl((val1), (val2),                     \

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp (right):

https://codereview.chromium.org/2738863002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp:758:
DCHECK(currentTime == minimumTime);
On 2017/03/14 22:33:14, tkent wrote:
> Use DCHECK_EQ if possible

It throws compiler error,
../../base/logging.h:659:3: error: no matching function for call to
'MakeCheckOpValueString'
  MakeCheckOpValueString(&ss, v1);
  ^~~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:703:1: note: in instantiation of function template
specialization 'logging::MakeCheckOpString<blink::SMILTime, blink::SMILTime>'
requested here
DEFINE_CHECK_OP_IMPL(EQ, ==)
^
../../base/logging.h:695:25: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
      return ::logging::MakeCheckOpString(v1, v2, names);                    \
                        ^
../../third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp:758:3:
note: in instantiation of function template specialization
'logging::CheckEQImpl<blink::SMILTime, blink::SMILTime>' requested here
  DCHECK_EQ(currentTime, minimumTime);
  ^
../../base/logging.h:899:31: note: expanded from macro 'DCHECK_EQ'
#define DCHECK_EQ(val1, val2) DCHECK_OP(EQ, ==, val1, val2)
                              ^
../../base/logging.h:853:18: note: expanded from macro 'DCHECK_OP'
      ::logging::Check##name##Impl((val1), (val2),                     \

[tkent, 2017-03-14 23:54:36]

On 2017/03/14 at 23:52:14, mrunal.kapade wrote:
> I have made suggested changes but some encounter compiler error.

Yeah, they are expected.  So I wrote "if possible."  It's ok to use DCHECK() for
them.

[mrunal, 2017-03-15 00:11:01]

On 2017/03/14 23:54:36, tkent wrote:
> On 2017/03/14 at 23:52:14, mrunal.kapade wrote:
> > I have made suggested changes but some encounter compiler error.
> 
> Yeah, they are expected.  So I wrote "if possible."  It's ok to use DCHECK()
for
> them.

Does everything look ok to you then?

[tkent, 2017-03-15 00:16:21]

lgtm

[mrunal, 2017-03-15 00:23:08]

The CQ bit was checked by mrunal.kapade@intel.com

[mrunal, 2017-03-15 00:23:09]

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/2738863002/#ps120001
(title: "Split DCHECKS wherever possible")

[commit-bot: I haz the power, 2017-03-15 00:23: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-15 02:05:10]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1489537388987700,
"parent_rev": "3f89cb4d2e652514501cd26bcf7fd75f1fda73f8", "commit_rev":
"24cf3269c241270f97890a977a3989577b2c4d31"}

[commit-bot: I haz the power, 2017-03-15 02:05:59]

Description was changed from

==========
Replace ASSERT with DCHECK in core/svg/

BUG=697687
==========

to

==========
Replace ASSERT with DCHECK in core/svg/

BUG=697687

Review-Url: https://codereview.chromium.org/2738863002
Cr-Commit-Position: refs/heads/master@{#456946}
Committed:
https://chromium.googlesource.com/chromium/src/+/24cf3269c241270f97890a977a39...
==========

[commit-bot: I haz the power, 2017-03-15 02:06:01]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/24cf3269c241270f97890a977a39...