Replace ASSERT, RELEASE_ASSERT, and ASSERT_NOT_REACHED in bindings

Replacing ASSERT, ASSERT_NOT_REACHED, and RELEASE_ASSERT in bindings 
with DCHECK_op, NOTREACHED,CHECK_op respectively 

BUG=708056
Review-Url: https://codereview.chromium.org/2817533003
Cr-Commit-Position: refs/heads/master@{#465109}
Committed: https://chromium.googlesource.com/chromium/src/+/0f6dc1c0458928b9e3abe4927ee99018e6945737

[bhagirathi.s, 2017-04-12 10:32:55]

Description was changed from

==========
Replace ASSERT, RELEASE_ASSERT, and ASSERT_NOT_REACHED in bindings

BUG=708056
==========

to

==========
Replacing ASSERT, ASSERT_NOT_REACHED, and RELEASE_ASSERT in bindings with
DCHECK_op, NOTREACHED,CHECK_op respectively 

BUG=708056
Review-Url: https://codereview.chromium.org/2817533003
==========

[bhagirathi.s, 2017-04-12 10:54:23]

bhagirathi.s@samsung.com changed reviewers:
+ srirama.m@samsung.com

[bhagirathi.s, 2017-04-12 10:54:24]

PTAL

[Srirama, 2017-04-12 15:00:05]

Peer review looks good. Just make sure the commit msg is shorter than 80chars on
each line.

[bhagirathi.s, 2017-04-13 04:54:56]

Description was changed from

==========
Replacing ASSERT, ASSERT_NOT_REACHED, and RELEASE_ASSERT in bindings with
DCHECK_op, NOTREACHED,CHECK_op respectively 

BUG=708056
Review-Url: https://codereview.chromium.org/2817533003
==========

to

==========
Replacing ASSERT, ASSERT_NOT_REACHED, and RELEASE_ASSERT in bindings 
with DCHECK_op, NOTREACHED,CHECK_op respectively 

BUG=708056
Review-Url: https://codereview.chromium.org/2817533003
==========

[bhagirathi.s, 2017-04-13 05:30:15]

bhagirathi.s@samsung.com changed reviewers:
+ jbroman@chromium.org, tkent@chromium.org

[bhagirathi.s, 2017-04-13 05:30:16]

PTAL

[tkent, 2017-04-13 05:37:16]

https://codereview.chromium.org/2817533003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/RejectedPromises.cpp (right):

https://codereview.chromium.org/2817533003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/RejectedPromises.cpp:130:
DCHECK(!promise_.IsEmpty() && !promise_.IsWeak());
Split this into two DCHECKs.

DCHECK(!promise_.IsEmpty());
DCHECK(!promise_.IsWeak());

https://codereview.chromium.org/2817533003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/RejectedPromises.cpp:136:
DCHECK(!promise_.IsEmpty() && promise_.IsWeak());
Ditto.

https://codereview.chromium.org/2817533003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp (right):

https://codereview.chromium.org/2817533003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:643: DCHECK(i < n - 1);
use DCHECK_LT

[bhagirathi.s, 2017-04-13 09:10:35]

https://codereview.chromium.org/2817533003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/RejectedPromises.cpp (right):

https://codereview.chromium.org/2817533003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/RejectedPromises.cpp:130:
DCHECK(!promise_.IsEmpty() && !promise_.IsWeak());
On 2017/04/13 05:37:16, tkent wrote:
> Split this into two DCHECKs.
> 
> DCHECK(!promise_.IsEmpty());
> DCHECK(!promise_.IsWeak());

Done.

https://codereview.chromium.org/2817533003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/RejectedPromises.cpp:136:
DCHECK(!promise_.IsEmpty() && promise_.IsWeak());
On 2017/04/13 05:37:16, tkent wrote:
> Ditto.

Done.

https://codereview.chromium.org/2817533003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp (right):

https://codereview.chromium.org/2817533003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:643: DCHECK(i < n - 1);
On 2017/04/13 05:37:16, tkent wrote:
> use DCHECK_LT

Done.

[tkent, 2017-04-13 09:21:03]

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

[commit-bot: I haz the power, 2017-04-13 09:21:28]

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-04-13 09:25:35]

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

[commit-bot: I haz the power, 2017-04-13 09:25:36]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  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-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tkent, 2017-04-13 09:30:47]

please rebase.

[bhagirathi.s, 2017-04-13 10:18:05]

Rebased on latest. PTAL

[tkent, 2017-04-13 10:19:41]

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

[commit-bot: I haz the power, 2017-04-13 10:19:56]

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-04-13 10:51:07]

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

[commit-bot: I haz the power, 2017-04-13 10:51:08]

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-...)

[bhagirathi.s, 2017-04-13 14:04:40]

DOMWrapperWorld.cpp is failed to compile in build bot, since IsIsolatedWorldId()
is defined under flag DCHECK_IS_ON() and DCHECK_IS_ON() is '0' in case of
release build. Shall I add DCHECK(IsIsolatedWorldId(world_id)) under flag
DCHECK_IS_ON() like below. Please suggest. 

#if DCHECK_IS_ON() 
DCHECK(IsIsolatedWorldId(world_id)); 
#endif

https://codereview.chromium.org/2817533003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp (right):

https://codereview.chromium.org/2817533003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp:190:
DCHECK(IsIsolatedWorldId(world_id));
This is failed to compile in build bot, since IsIsolatedWorldId() is defined
under flag DCHECK_IS_ON() and DCHECK_IS_ON() is '0' in case of release build.
Shall I add DCHECK(IsIsolatedWorldId(world_id)) under flag DCHECK_IS_ON() like
below. Please suggest.

#if DCHECK_IS_ON()
DCHECK(IsIsolatedWorldId(world_id));
#endif

[tkent, 2017-04-14 00:26:45]

https://codereview.chromium.org/2817533003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp (right):

https://codereview.chromium.org/2817533003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp:190:
DCHECK(IsIsolatedWorldId(world_id));
On 2017/04/13 at 14:04:39, bhagirathi.s wrote:
> This is failed to compile in build bot, since IsIsolatedWorldId() is defined
under flag DCHECK_IS_ON() and DCHECK_IS_ON() is '0' in case of release build.
Shall I add DCHECK(IsIsolatedWorldId(world_id)) under flag DCHECK_IS_ON() like
below. Please suggest.
> 
> #if DCHECK_IS_ON()
> DCHECK(IsIsolatedWorldId(world_id));
> #endif

Yes, it's a straight-forward fix for this kind of issue.


Another solution would be to define IsIsolatedWorldId() in !DCHECK_IS_ON() too. 
e.g.

static bool IsIsolatedWorldId(int world_id) {
#if DCHECK_IS_ON()
  return DOMWrapperWorld::kMainWorldId < world_id &&
         world_id < DOMWrapperWorld::kIsolatedWorldIdLimit;
#else
  NOTREACHED() << "This function should be used only in DCHECK.";
  return true;
#endif
}

[Srirama, 2017-04-17 07:06:49]

The CQ bit was checked by srirama.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 07:06:57]

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

[bhagirathi.s, 2017-04-17 09:13:42]

PTAL

https://codereview.chromium.org/2817533003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp (right):

https://codereview.chromium.org/2817533003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp:190:
DCHECK(IsIsolatedWorldId(world_id));
On 2017/04/14 00:26:44, tkent wrote:
> On 2017/04/13 at 14:04:39, bhagirathi.s wrote:
> > This is failed to compile in build bot, since IsIsolatedWorldId() is defined
> under flag DCHECK_IS_ON() and DCHECK_IS_ON() is '0' in case of release build.
> Shall I add DCHECK(IsIsolatedWorldId(world_id)) under flag DCHECK_IS_ON() like
> below. Please suggest.
> > 
> > #if DCHECK_IS_ON()
> > DCHECK(IsIsolatedWorldId(world_id));
> > #endif
> 
> Yes, it's a straight-forward fix for this kind of issue.
> 
> 
> Another solution would be to define IsIsolatedWorldId() in !DCHECK_IS_ON()
too. 
> e.g.
> 
> static bool IsIsolatedWorldId(int world_id) {
> #if DCHECK_IS_ON()
>   return DOMWrapperWorld::kMainWorldId < world_id &&
>          world_id < DOMWrapperWorld::kIsolatedWorldIdLimit;
> #else
>   NOTREACHED() << "This function should be used only in DCHECK.";
>   return true;
> #endif
> }
> 

DCHECK_IS_ON() added to fix the compilation error.

[bhagirathi.s, 2017-04-17 09:16:39]

Description was changed from

==========
Replacing ASSERT, ASSERT_NOT_REACHED, and RELEASE_ASSERT in bindings 
with DCHECK_op, NOTREACHED,CHECK_op respectively 

BUG=708056
Review-Url: https://codereview.chromium.org/2817533003
==========

to

==========
Replacing ASSERT, ASSERT_NOT_REACHED, and RELEASE_ASSERT in bindings 
with DCHECK_op, NOTREACHED,CHECK_op respectively 

BUG=708056
==========

[commit-bot: I haz the power, 2017-04-17 09:18:20]

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

[commit-bot: I haz the power, 2017-04-17 09:18:21]

Dry run: This issue passed the CQ dry run.

[tkent, 2017-04-18 01:53:32]

The CQ bit was checked by tkent@chromium.org

[tkent, 2017-04-18 01:53:33]

lgtm

[commit-bot: I haz the power, 2017-04-18 01:54:19]

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-04-18 02:36:32]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492480412931200,
"parent_rev": "0c3532027893052475cd87ac0f6c758046f34415", "commit_rev":
"0f6dc1c0458928b9e3abe4927ee99018e6945737"}

[commit-bot: I haz the power, 2017-04-18 02:36:45]

Description was changed from

==========
Replacing ASSERT, ASSERT_NOT_REACHED, and RELEASE_ASSERT in bindings 
with DCHECK_op, NOTREACHED,CHECK_op respectively 

BUG=708056
==========

to

==========
Replacing ASSERT, ASSERT_NOT_REACHED, and RELEASE_ASSERT in bindings 
with DCHECK_op, NOTREACHED,CHECK_op respectively 

BUG=708056

Review-Url: https://codereview.chromium.org/2817533003
Cr-Commit-Position: refs/heads/master@{#465109}
Committed:
https://chromium.googlesource.com/chromium/src/+/0f6dc1c0458928b9e3abe4927ee9...
==========

[commit-bot: I haz the power, 2017-04-18 02:36:46]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/0f6dc1c0458928b9e3abe4927ee9...