Replace ASSERT with DCHECK in core/

Replace ASSERT with DCHECK in core/

This CL only affects instances of ASSERT() that have unary parameters
with no (in)equality checks.

BUG=596760
Review-Url: https://codereview.chromium.org/2858963002
Cr-Commit-Position: refs/heads/master@{#470199}
Committed: https://chromium.googlesource.com/chromium/src/+/a30679c30fc4d5074fe2f7ff1d27a404fca84ff6

[pilgrim_google, 2017-05-03 19:14:29]

pilgrim@chromium.org changed reviewers:
+ haraken@chromium.org, tkent@chromium.org

[pilgrim_google, 2017-05-03 19:14:39]

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

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

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-05-03 20:00:45]

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

[commit-bot: I haz the power, 2017-05-03 20:00:46]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[haraken, 2017-05-04 01:41:10]

LGTM

[pilgrim_google, 2017-05-04 13:01:46]

On 2017/05/04 at 01:41:10, haraken wrote:
> LGTM

This error on chromeos_amd64-generic_chromium_compile_only_ng worries me a
little:

FAILED: obj/third_party/WebKit/Source/core/clipboard/clipboard/DataObject.o 
/b/c/goma_client/gomacc x86_64-cros-linux-gnu-clang++
-B/b/c/b/chromeos_amd64_generic_chromium_compile_only_ng/.cros_cache/chrome-sdk/tarballs/amd64-generic+9510.0.0+target_toolchain/usr/x86_64-pc-linux-gnu/x86_64-cros-linux-gnu/binutils-bin/2.25.51-gold
-Wno-unknown-warning-option -MMD -MF
obj/third_party/WebKit/Source/core/clipboard/clipboard/DataObject.o.d
-DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_ASH=1 -DUSE_AURA=1 -DUSE_NSS_CERTS=1
-DUSE_OZONE=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL
-DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=\"300839-1\"
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DOS_CHROMEOS -DNDEBUG
-DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DBLINK_CORE_IMPLEMENTATION=1
-DUSE_EGL -DBLINK_IMPLEMENTATION=1 -DINSIDE_BLINK
-DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_SUPPORT_GPU=1
-DWTF_USE_WEBAUDIO_FFMPEG=1 -DWTF_USE_DEFAULT_RENDER_THEME=1
-DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION
-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DLIBXSLT_STATIC -I../.. -Igen
-I../../third_party/WebKit/Source -I../../third_party/WebKit -Igen/blink
-Igen/third_party/WebKit -I../../third_party/khronos -I../../gpu
-I../../third_party/libwebp -I../../third_party/WebKit/Source -Igen/blink
-I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c
-I../../third_party/skia/include/config -I../../third_party/skia/include/core
-I../../third_party/skia/include/effects -I../../third_party/skia/include/images
-I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops
-I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe
-I../../third_party/skia/include/ports -I../../third_party/skia/include/utils
-I../../third_party/skia/third_party/vulkan -I../../third_party/skia/include/gpu
-I../../third_party/skia/src/gpu -I../../third_party/skia/src/sksl
-I../../third_party/icu/source/common -I../../third_party/icu/source/i18n
-I../../third_party/angle/include
-I../../third_party/angle/src/common/third_party/numerics -Igen/angle
-I../../third_party/iccjpeg -I../../third_party/libpng -I../../third_party/zlib
-I../../third_party/libxml/src/include -I../../third_party/libxml/linux/include
-I../../third_party/libxslt -I../../third_party/ots/include
-I../../third_party/snappy/src -I../../third_party/snappy/linux
-I../../v8/include -Igen/v8/include -fno-strict-aliasing
-Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__=
-funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin
-fcolor-diagnostics -m64 -march=x86-64 -pthread -Wall -Wextra
-Wno-missing-field-initializers -Wno-unused-parameter -Wno-shift-negative-value
-Wno-c++11-narrowing -Wno-covered-switch-default
-Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override
-Wno-undefined-var-template -Wno-nonportable-include-path
-Wno-address-of-packed-member -Wno-unused-lambda-capture
-Wno-user-defined-warnings -O2 -fno-ident -fdata-sections -ffunction-sections
-fno-omit-frame-pointer -g2 -gsplit-dwarf
--sysroot=../../../.cros_cache/chrome-sdk/tarballs/amd64-generic+9510.0.0+sysroot_chromeos-base_chromeos-chrome.tar.xz
-fvisibility=hidden -Wheader-hygiene -Wstring-conversion
-Wtautological-overlap-compare -Wexit-time-destructors -Wglobal-constructors
-DLIBXML_STATIC= -fvisibility-inlines-hidden -std=gnu++11 -fno-rtti
-fno-exceptions -pipe -pipe -pipe -march=x86-64 -msse3 -fno-split-dwarf-inlining
-D__google_stl_debug_vector=1 -Wno-unknown-warning-option -Wno-inline-asm -c
../../third_party/WebKit/Source/core/clipboard/DataObject.cpp -o
obj/third_party/WebKit/Source/core/clipboard/clipboard/DataObject.o
../../third_party/WebKit/Source/core/clipboard/DataObject.cpp:60:12: error: use
of undeclared identifier 'types_seen'
    DCHECK(types_seen.insert(type).is_new_entry);
           ^
../../third_party/WebKit/Source/core/clipboard/DataObject.cpp:145:16: error: use
of undeclared identifier 'types_seen'
        DCHECK(types_seen.insert(item->GetType()).is_new_entry);
               ^
../../third_party/WebKit/Source/core/clipboard/DataObject.cpp:154:12: error: use
of undeclared identifier 'types_seen'
    DCHECK(types_seen.insert(kMimeTypeFiles).is_new_entry);
           ^
3 errors generated.

The function in question has a variable that is only declared if DCHECK is on:

#if DCHECK_IS_ON()
  HashSet<String> types_seen;
#endif
.
.
.
DCHECK(types_seen.insert(type).is_new_entry);

Is this a false positive, or what should we do about it?

[haraken, 2017-05-04 14:30:54]

On 2017/05/04 13:01:46, pilgrim_google wrote:
> On 2017/05/04 at 01:41:10, haraken wrote:
> > LGTM
> 
> This error on chromeos_amd64-generic_chromium_compile_only_ng worries me a
> little:
> 
> FAILED: obj/third_party/WebKit/Source/core/clipboard/clipboard/DataObject.o 
> /b/c/goma_client/gomacc x86_64-cros-linux-gnu-clang++
>
-B/b/c/b/chromeos_amd64_generic_chromium_compile_only_ng/.cros_cache/chrome-sdk/tarballs/amd64-generic+9510.0.0+target_toolchain/usr/x86_64-pc-linux-gnu/x86_64-cros-linux-gnu/binutils-bin/2.25.51-gold
> -Wno-unknown-warning-option -MMD -MF
> obj/third_party/WebKit/Source/core/clipboard/clipboard/DataObject.o.d
> -DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_ASH=1 -DUSE_AURA=1
-DUSE_NSS_CERTS=1
> -DUSE_OZONE=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD
-DSAFE_BROWSING_DB_LOCAL
> -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=\"300839-1\"
> -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DOS_CHROMEOS -DNDEBUG
> -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DBLINK_CORE_IMPLEMENTATION=1
> -DUSE_EGL -DBLINK_IMPLEMENTATION=1 -DINSIDE_BLINK
> -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_SUPPORT_GPU=1
> -DWTF_USE_WEBAUDIO_FFMPEG=1 -DWTF_USE_DEFAULT_RENDER_THEME=1
> -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION
> -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DLIBXSLT_STATIC -I../.. -Igen
> -I../../third_party/WebKit/Source -I../../third_party/WebKit -Igen/blink
> -Igen/third_party/WebKit -I../../third_party/khronos -I../../gpu
> -I../../third_party/libwebp -I../../third_party/WebKit/Source -Igen/blink
> -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c
> -I../../third_party/skia/include/config -I../../third_party/skia/include/core
> -I../../third_party/skia/include/effects
-I../../third_party/skia/include/images
> -I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops
> -I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe
> -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils
> -I../../third_party/skia/third_party/vulkan
-I../../third_party/skia/include/gpu
> -I../../third_party/skia/src/gpu -I../../third_party/skia/src/sksl
> -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n
> -I../../third_party/angle/include
> -I../../third_party/angle/src/common/third_party/numerics -Igen/angle
> -I../../third_party/iccjpeg -I../../third_party/libpng
-I../../third_party/zlib
> -I../../third_party/libxml/src/include
-I../../third_party/libxml/linux/include
> -I../../third_party/libxslt -I../../third_party/ots/include
> -I../../third_party/snappy/src -I../../third_party/snappy/linux
> -I../../v8/include -Igen/v8/include -fno-strict-aliasing
> -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__=
> -funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin
> -fcolor-diagnostics -m64 -march=x86-64 -pthread -Wall -Wextra
> -Wno-missing-field-initializers -Wno-unused-parameter
-Wno-shift-negative-value
> -Wno-c++11-narrowing -Wno-covered-switch-default
> -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override
> -Wno-undefined-var-template -Wno-nonportable-include-path
> -Wno-address-of-packed-member -Wno-unused-lambda-capture
> -Wno-user-defined-warnings -O2 -fno-ident -fdata-sections -ffunction-sections
> -fno-omit-frame-pointer -g2 -gsplit-dwarf
>
--sysroot=../../../.cros_cache/chrome-sdk/tarballs/amd64-generic+9510.0.0+sysroot_chromeos-base_chromeos-chrome.tar.xz
> -fvisibility=hidden -Wheader-hygiene -Wstring-conversion
> -Wtautological-overlap-compare -Wexit-time-destructors -Wglobal-constructors
> -DLIBXML_STATIC= -fvisibility-inlines-hidden -std=gnu++11 -fno-rtti
> -fno-exceptions -pipe -pipe -pipe -march=x86-64 -msse3
-fno-split-dwarf-inlining
> -D__google_stl_debug_vector=1 -Wno-unknown-warning-option -Wno-inline-asm -c
> ../../third_party/WebKit/Source/core/clipboard/DataObject.cpp -o
> obj/third_party/WebKit/Source/core/clipboard/clipboard/DataObject.o
> ../../third_party/WebKit/Source/core/clipboard/DataObject.cpp:60:12: error:
use
> of undeclared identifier 'types_seen'
>     DCHECK(types_seen.insert(type).is_new_entry);
>            ^
> ../../third_party/WebKit/Source/core/clipboard/DataObject.cpp:145:16: error:
use
> of undeclared identifier 'types_seen'
>         DCHECK(types_seen.insert(item->GetType()).is_new_entry);
>                ^
> ../../third_party/WebKit/Source/core/clipboard/DataObject.cpp:154:12: error:
use
> of undeclared identifier 'types_seen'
>     DCHECK(types_seen.insert(kMimeTypeFiles).is_new_entry);
>            ^
> 3 errors generated.
> 
> The function in question has a variable that is only declared if DCHECK is on:
> 
> #if DCHECK_IS_ON()
>   HashSet<String> types_seen;
> #endif
> .
> .
> .
> DCHECK(types_seen.insert(type).is_new_entry);
> 
> Is this a false positive, or what should we do about it?

Ah, the contents of DCHECK is unconditionally compiled on some platforms, so
we'll need to drop the DCHECK_IS_ON from the declaration.

[haraken, 2017-05-04 14:31:51]

haraken@chromium.org changed reviewers:
+ dcheng@chromium.org

[haraken, 2017-05-04 14:31:52]

+dcheng for double-checking

Alternately we should add DCHECK_IS_ON to the DCHECKs. Then we don't need to
drop DCHECK_IS_ON from the variable declaration.

[pilgrim_google, 2017-05-04 15:08:44]

On 2017/05/04 at 14:31:52, haraken wrote:
> +dcheng for double-checking
> 
> Alternately we should add DCHECK_IS_ON to the DCHECKs. Then we don't need to
drop DCHECK_IS_ON from the variable declaration.

This seems like the safer course.

[dcheng, 2017-05-04 17:28:15]

On 2017/05/04 15:08:44, pilgrim_google wrote:
> On 2017/05/04 at 14:31:52, haraken wrote:
> > +dcheng for double-checking
> > 
> > Alternately we should add DCHECK_IS_ON to the DCHECKs. Then we don't need to
> drop DCHECK_IS_ON from the variable declaration.
> 
> This seems like the safer course.

Another way of doing this is DLOG_IF(FATAL, condition)... not quite as concise,
though it will avoid evaluating condition at all then.

The business with DCHECK has been troublesome enough that I'm wondering if we
should just not evaluate DCHECK's condition at all when DCHECKS aren't
enabled...

[pilgrim_google, 2017-05-04 17:42:24]

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

[commit-bot: I haz the power, 2017-05-04 17:43:04]

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-05-04 18:13:52]

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

[commit-bot: I haz the power, 2017-05-04 18:13:54]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[pilgrim_google, 2017-05-04 19:24:38]

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

[commit-bot: I haz the power, 2017-05-04 19:25:16]

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-05-04 19:39:20]

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

[commit-bot: I haz the power, 2017-05-04 19:39:21]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[pilgrim_google, 2017-05-04 19:44:50]

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

[commit-bot: I haz the power, 2017-05-04 19:45:30]

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-05-04 20:17:38]

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

[commit-bot: I haz the power, 2017-05-04 20:17:39]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[pilgrim_google, 2017-05-05 00:39:25]

The CQ bit was checked by pilgrim@chromium.org

[pilgrim_google, 2017-05-05 00:39:26]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2858963002/#ps60001
(title: "fix guard in DataObject")

[commit-bot: I haz the power, 2017-05-05 00:39:50]

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-05-05 01:20:12]

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

[commit-bot: I haz the power, 2017-05-05 01:20:17]

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

[pilgrim_google, 2017-05-05 17:02:43]

The CQ bit was checked by pilgrim@chromium.org

[pilgrim_google, 2017-05-05 17:02:44]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2858963002/#ps80001
(title: "add DCHECK_IS_ON guard in FrameView")

[commit-bot: I haz the power, 2017-05-05 17:03:13]

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-05-05 17:36:04]

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

[commit-bot: I haz the power, 2017-05-05 17:36:06]

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

[pilgrim_google, 2017-05-05 17:57:56]

The CQ bit was checked by pilgrim@chromium.org

[pilgrim_google, 2017-05-05 17:57:56]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2858963002/#ps100001
(title: "add guard in WorkerBackingThread")

[commit-bot: I haz the power, 2017-05-05 17:58:36]

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-05-05 18:29:17]

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

[commit-bot: I haz the power, 2017-05-05 18:29:19]

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

[pilgrim_google, 2017-05-08 20:18:26]

The CQ bit was checked by pilgrim@chromium.org

[pilgrim_google, 2017-05-08 20:18:29]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2858963002/#ps120001
(title: "WorkerBackingThread")

[commit-bot: I haz the power, 2017-05-08 20:19:22]

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-05-09 04:22:51]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1494274706406220,
"parent_rev": "c4c5806ef59df14009ad9258ad98d231c5fe1699", "commit_rev":
"a30679c30fc4d5074fe2f7ff1d27a404fca84ff6"}

[commit-bot: I haz the power, 2017-05-09 04:23:02]

Description was changed from

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

This CL only affects instances of ASSERT() that have unary parameters
with no (in)equality checks.

BUG=596760
==========

to

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

This CL only affects instances of ASSERT() that have unary parameters
with no (in)equality checks.

BUG=596760

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

[commit-bot: I haz the power, 2017-05-09 04:23:04]

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