Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2541633002:
Convert GraphicsLayer from ASSERT to DCHECK (Closed)
|
Created:
4 years ago by Stephen Chennney Modified:
4 years ago Reviewers:
chrishtr CC:
ajuma+watch_chromium.org, blink-layers+watch_chromium.org, blink-reviews, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, f(malita), jbroman, Justin Novosad, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionConvert GraphicsLayer from ASSERT to DCHECK
Presubmit is complaining about this file, so convert it now as
a separate change. Also add missing braces.
R=chrishtr@chromium.org
BUG=666660
Committed: https://crrev.com/debb83ad61b8ba4beb3e0508e06e2cb4e0a9e25f
Cr-Commit-Position: refs/heads/master@{#435275}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Fix comment #
Total comments: 4
Patch Set 3 : Address review comments #
Messages
Total messages: 27 (16 generated)
I hit this trying to upload a rebased composited border radius patch. Seems best to just fix it. https://codereview.chromium.org/2541633002/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp (right): https://codereview.chromium.org/2541633002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:168: #endif The way to handle this is taken from https://codereview.chromium.org/2530493002 I need to fix it's -> its
Description was changed from ========== Convert GraphicsLayer from ASSERT to DCHECK Presubmit is complaining about this file, so convert it now as a separate change. Also add missing braces. R=chrishtr@chromium.org BUG= ========== to ========== Convert GraphicsLayer from ASSERT to DCHECK Presubmit is complaining about this file, so convert it now as a separate change. Also add missing braces. R=chrishtr@chromium.org BUG=666660 ==========
The CQ bit was checked by schenney@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
https://codereview.chromium.org/2541633002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp (right): https://codereview.chromium.org/2541633002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:166: #if DCHECK_IS_ON() // DCHECK references its condition even when not enabled I don't think the comment is needed. This is a known issue, referenced on blink-dev just last week. https://codereview.chromium.org/2541633002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:1186: CHECK(drawsContent()); Let's leave this one. It's called may times per frame. Apparently CHECK is slower than RELEASE_ASSERT on Windows.
I've fixed up based on the comments. Thanks. https://codereview.chromium.org/2541633002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp (right): https://codereview.chromium.org/2541633002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:166: #if DCHECK_IS_ON() // DCHECK references its condition even when not enabled On 2016/11/29 21:13:59, chrishtr wrote: > I don't think the comment is needed. This is a known issue, referenced on > blink-dev just last week. Done. https://codereview.chromium.org/2541633002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:1186: CHECK(drawsContent()); On 2016/11/29 21:13:59, chrishtr wrote: > Let's leave this one. It's called may times per frame. Apparently CHECK is > slower than RELEASE_ASSERT on Windows. Done.
The CQ bit was checked by schenney@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by schenney@chromium.org
The CQ bit was checked by schenney@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by schenney@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by schenney@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480522012367500,
"parent_rev": "4babd412add567ba0d19accb33eb6bacc1ba8061", "commit_rev":
"951750fb3d9483e37b30a316ad89a93725ddfe0c"}
Message was sent while issue was closed.
Description was changed from ========== Convert GraphicsLayer from ASSERT to DCHECK Presubmit is complaining about this file, so convert it now as a separate change. Also add missing braces. R=chrishtr@chromium.org BUG=666660 ========== to ========== Convert GraphicsLayer from ASSERT to DCHECK Presubmit is complaining about this file, so convert it now as a separate change. Also add missing braces. R=chrishtr@chromium.org BUG=666660 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Convert GraphicsLayer from ASSERT to DCHECK Presubmit is complaining about this file, so convert it now as a separate change. Also add missing braces. R=chrishtr@chromium.org BUG=666660 ========== to ========== Convert GraphicsLayer from ASSERT to DCHECK Presubmit is complaining about this file, so convert it now as a separate change. Also add missing braces. R=chrishtr@chromium.org BUG=666660 Committed: https://crrev.com/debb83ad61b8ba4beb3e0508e06e2cb4e0a9e25f Cr-Commit-Position: refs/heads/master@{#435275} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/debb83ad61b8ba4beb3e0508e06e2cb4e0a9e25f Cr-Commit-Position: refs/heads/master@{#435275} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
