Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(31)

Issue 2581733002: Fix tautological comparison and add clang flag (Closed)

Created:
11 months, 1 week ago by altimin
Modified:
11 months ago
Reviewers:
michaeln, brettw, jsbell, tzik
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix tautological comparison and add clang flag Committed: https://crrev.com/1b142eb67aedcd40f07234e38174b5b0f9995dc5 Cr-Commit-Position: refs/heads/master@{#439346}

Patch Set 1 #

Total comments: 3

Patch Set 2 : Remove warnings from openh264 #

Patch Set 3 : Remove clang warnings only when using clang #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -3 lines) Patch
M build/config/clang/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M storage/browser/quota/quota_manager.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M third_party/openh264/BUILD.gn View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
Trybot results:  win_clang   android_clang_dbg_recipe   android_n5x_swarming_rel   linux_android_rel_ng   mac_chromium_rel_ng   linux_chromium_rel_ng   linux_chromium_compile_dbg_ng   win_chromium_x64_rel_ng   win_chromium_rel_ng   cast_shell_linux   mac_chromium_compile_dbg_ng   cast_shell_android   ios-simulator   android_arm64_dbg_recipe   ios-device   ios-device-xcode-clang   ios-simulator-xcode-clang   win_chromium_compile_dbg_ng   android_compile_dbg   chromeos_daisy_chromium_compile_only_ng   chromeos_amd64-generic_chromium_compile_only_ng   android_cronet   linux_chromium_rel_ng   chromium_presubmit   linux_chromium_rel_ng   linux_chromium_rel_ng   linux_chromium_chromeos_rel_ng   linux_chromium_chromeos_ozone_rel_ng   chromium_presubmit   linux_chromium_asan_rel_ng   linux_chromium_rel_ng   linux_chromium_chromeos_rel_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_asan_rel_ng   chromium_presubmit   android_n5x_swarming_rel   linux_android_rel_ng   mac_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-simulator-xcode-clang   ios-simulator   ios-device-xcode-clang   ios-device   win_clang   win_chromium_x64_rel_ng   win_chromium_rel_ng   win_chromium_compile_dbg_ng   linux_chromium_rel_ng   linux_chromium_compile_dbg_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_chromeos_rel_ng   chromium_presubmit   linux_chromium_asan_rel_ng   chromeos_amd64-generic_chromium_compile_only_ng   chromeos_daisy_chromium_compile_only_ng   cast_shell_linux   linux_android_rel_ng   cast_shell_android   android_n5x_swarming_rel   android_cronet   android_compile_dbg   android_clang_dbg_recipe   android_arm64_dbg_recipe 

Messages

Total messages: 39 (24 generated)
altimin
PTAL
11 months, 1 week ago (2016-12-15 17:55:04 UTC) #2
brettw
build owners lgtm, but please check with Michael before checking in. https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota_manager.cc File storage/browser/quota/quota_manager.cc (right): ...
11 months, 1 week ago (2016-12-15 18:49:31 UTC) #3
altimin
michaeln@, could you PTAL? https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota_manager.cc File storage/browser/quota/quota_manager.cc (right): https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota_manager.cc#newcode89 storage/browser/quota/quota_manager.cc:89: return type == kStorageTypeTemporary || ...
11 months, 1 week ago (2016-12-15 18:57:08 UTC) #5
michaeln
https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota_manager.cc File storage/browser/quota/quota_manager.cc (right): https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota_manager.cc#newcode89 storage/browser/quota/quota_manager.cc:89: return type == kStorageTypeTemporary || type == kStorageTypePersistent; On ...
11 months, 1 week ago (2016-12-15 19:26:55 UTC) #10
michaeln
> I wouldn't have expected persistent storage > to be allowed in incognito. is is ...
11 months, 1 week ago (2016-12-15 19:29:40 UTC) #11
altimin
brettw@, there were failures in openh264, I disabled warnings for that third-party library. Could you ...
11 months, 1 week ago (2016-12-15 20:51:19 UTC) #12
brettw
lgtm
11 months, 1 week ago (2016-12-15 22:10:45 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581733002/30001
11 months, 1 week ago (2016-12-16 19:00:42 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/281306) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
11 months, 1 week ago (2016-12-16 22:21:33 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581733002/30001
11 months, 1 week ago (2016-12-16 23:43:07 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/358626)
11 months, 1 week ago (2016-12-17 03:46:26 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581733002/30001
11 months, 1 week ago (2016-12-17 14:23:12 UTC) #32
commit-bot: I haz the power
Committed patchset #3 (id:30001)
11 months, 1 week ago (2016-12-17 18:51:37 UTC) #35
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/1b142eb67aedcd40f07234e38174b5b0f9995dc5 Cr-Commit-Position: refs/heads/master@{#439346}
11 months, 1 week ago (2016-12-17 18:54:21 UTC) #37
jsbell
11 months ago (2016-12-20 19:46:35 UTC) #39
Message was sent while issue was closed.
FYI: pending reversion for the base CL at
https://codereview.chromium.org/2592793002

That'll remove the tautological statement; we'll need to make sure this fix gets
reapplied when it relands although the clang flag should catch it.

Powered by Google App Engine
This is Rietveld efc10ee0f