Use LOG() or LOG_IF() instead of IMMEDIATE_CRASH

Use LOG(FATAL) or CHECK*() instead of IMMEDIATE_CRASH

Apply review comments in issue 2919443002. Use LOG() and CHECK*() to simplify
the code.

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

[gyuyoung, 2017-06-01 04:24:00]

gyuyoung@igalia.com changed reviewers:
+ haraken@chromium.org, tkent@chromium.org

[gyuyoung, 2017-06-01 04:24:00]

Hi tkent, this CL is to fix it you pointed out in issue 2919443002.

[tkent, 2017-06-02 00:26:49]

I changed my mind.  Maybe CHECK() is simpler than LOG_IF(FATAL).  I'm sorry for
the extra iteration.

So, LOG_IF(FATAL, condition) should be CHECK(!condition).

https://codereview.chromium.org/2919653004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/wtf/AssertionsTest.cpp (right):

https://codereview.chromium.org/2919653004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/wtf/AssertionsTest.cpp:31:
EXPECT_DEATH_IF_SUPPORTED(LOG(FATAL), "");
LOG() is defined by base/logging.*, Blink doesn't need to have unittest for it.

[gyuyoung, 2017-06-02 14:42:10]

Patchset #4 (id:60001) has been deleted

[gyuyoung, 2017-06-02 14:42:16]

Patchset #3 (id:40001) has been deleted

[gyuyoung, 2017-06-02 14:45:46]

https://codereview.chromium.org/2919653004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/wtf/AssertionsTest.cpp (right):

https://codereview.chromium.org/2919653004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/wtf/AssertionsTest.cpp:31:
EXPECT_DEATH_IF_SUPPORTED(LOG(FATAL), "");
On 2017/06/02 00:26:49, tkent wrote:
> LOG() is defined by base/logging.*, Blink doesn't need to have unittest for
it.

Done.

[gyuyoung, 2017-06-02 14:46:44]

On 2017/06/02 00:26:49, tkent wrote:
> I changed my mind.  Maybe CHECK() is simpler than LOG_IF(FATAL).  I'm sorry
for
> the extra iteration.
> 
> So, LOG_IF(FATAL, condition) should be CHECK(!condition).

I try to use CHECK() instead of LOG_IF() as you suggested. Please check if there
is any wrong use.

[tkent, 2017-06-05 01:58:51]

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right):

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutTableCell.h:106:
CHECK(!UNLIKELY(column > kMaxColumnIndex));
UNLIKELY isn't necessary. CHECK() contains it internally.

CHECK_LE(column, kMaxColumnIndex);

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutTableRow.h (right):

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutTableRow.h:88:
CHECK(!UNLIKELY(row_index > kMaxRowIndex));
UNLIKELY isn't necessary.  CHECK() contains it internally.

CHECK_LE(row_index, kMaxRowIndex);

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/wtf/Assertions.h (right):

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/wtf/Assertions.h:132: CHECK(condition) <<
"Security DCHECK failed: " #condition ". "
Do not replace this.  LOG_IF and CHECK produce difference messages, and I think
ClusterFuzz replies on the message.

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/wtf/Assertions.h:135: CHECK(condition) <<
"Security CHECK failed: " #condition ". "
Ditto.

[gyuyoung, 2017-06-05 02:32:30]

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/wtf/Assertions.h (right):

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/wtf/Assertions.h:132: CHECK(condition) <<
"Security DCHECK failed: " #condition ". "
On 2017/06/05 01:58:51, tkent wrote:
> Do not replace this.  LOG_IF and CHECK produce difference messages, and I
think
> ClusterFuzz replies on the message.

Do you mean that LOG_IF() should be kept ?

[tkent, 2017-06-05 02:34:56]

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/wtf/Assertions.h (right):

https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/wtf/Assertions.h:132: CHECK(condition) <<
"Security DCHECK failed: " #condition ". "
On 2017/06/05 at 02:32:29, gyuyoung wrote:
> On 2017/06/05 01:58:51, tkent wrote:
> > Do not replace this.  LOG_IF and CHECK produce difference messages, and I
think
> > ClusterFuzz replies on the message.
> 
> Do you mean that LOG_IF() should be kept ?

Yes.

[gyuyoung, 2017-06-05 03:34:24]

On 2017/06/05 02:34:56, tkent wrote:
>
https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/platform/wtf/Assertions.h (right):
> 
>
https://codereview.chromium.org/2919653004/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/platform/wtf/Assertions.h:132: CHECK(condition) <<
> "Security DCHECK failed: " #condition ". "
> On 2017/06/05 at 02:32:29, gyuyoung wrote:
> > On 2017/06/05 01:58:51, tkent wrote:
> > > Do not replace this.  LOG_IF and CHECK produce difference messages, and I
> think
> > > ClusterFuzz replies on the message.
> > 
> > Do you mean that LOG_IF() should be kept ?
> 
> Yes.

All done. Thx.

[tkent, 2017-06-05 03:42:32]

Description was changed from

==========
Use LOG() or LOG_IF() instead of IMMEDIATE_CRASH

Apply review comments in issue 2919443002. To use LOG() and LOG_IF() use
fewer lines.

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

to

==========
Use LOG(FATAL) or CHECK*() instead of IMMEDIATE_CRASH

Apply review comments in issue 2919443002. Use LOG() and CHECK*() to simplify
the code.

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

[tkent, 2017-06-05 03:42:49]

lgtm

[gyuyoung, 2017-06-05 03:43:58]

The CQ bit was checked by gyuyoung@igalia.com

[commit-bot: I haz the power, 2017-06-05 03:44:06]

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-06-05 05:29:57]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1496634238597650,
"parent_rev": "e86efa750446d66832e1dd957be78f24a34e1ad9", "commit_rev":
"46740f4e99de22f90302e72ca3a91730521b5827"}

[commit-bot: I haz the power, 2017-06-05 05:30:10]

Description was changed from

==========
Use LOG(FATAL) or CHECK*() instead of IMMEDIATE_CRASH

Apply review comments in issue 2919443002. Use LOG() and CHECK*() to simplify
the code.

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

to

==========
Use LOG(FATAL) or CHECK*() instead of IMMEDIATE_CRASH

Apply review comments in issue 2919443002. Use LOG() and CHECK*() to simplify
the code.

BUG=596760

Review-Url: https://codereview.chromium.org/2919653004
Cr-Commit-Position: refs/heads/master@{#476946}
Committed:
https://chromium.googlesource.com/chromium/src/+/46740f4e99de22f90302e72ca3a9...
==========

[commit-bot: I haz the power, 2017-06-05 05:30:12]

Committed patchset #5 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/46740f4e99de22f90302e72ca3a9...