Fix tautological comparison and add clang flag

Fix tautological comparison and add clang flag

Committed: https://crrev.com/1b142eb67aedcd40f07234e38174b5b0f9995dc5
Cr-Commit-Position: refs/heads/master@{#439346}

[altimin, 2016-12-15 17:55:03]

altimin@chromium.org changed reviewers:
+ brettw@chromium.org, tzik@chromium.org

[altimin, 2016-12-15 17:55:04]

PTAL

[brettw, 2016-12-15 18:49:31]

build owners lgtm, but please check with Michael before checking in.

https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota...
File storage/browser/quota/quota_manager.cc (right):

https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota...
storage/browser/quota/quota_manager.cc:89: return type == kStorageTypeTemporary
|| type == kStorageTypePersistent;
Can you check with the original author, michaeln, about the changes to this
file? For this line in particular, I wouldn't have expected persistent storage
to be allowed in incognito.

[altimin, 2016-12-15 18:56:52]

altimin@chromium.org changed reviewers:
+ michaeln@chromium.org

[altimin, 2016-12-15 18:57:08]

michaeln@, could you PTAL?

https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota...
File storage/browser/quota/quota_manager.cc (right):

https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota...
storage/browser/quota/quota_manager.cc:89: return type == kStorageTypeTemporary
|| type == kStorageTypePersistent;
On 2016/12/15 18:49:31, brettw (ping after 24h) wrote:
> Can you check with the original author, michaeln, about the changes to this
> file? For this line in particular, I wouldn't have expected persistent storage
> to be allowed in incognito.

Sure, moved him from cc to reviewers.

[altimin, 2016-12-15 18:57:18]

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

[commit-bot: I haz the power, 2016-12-15 18:58:01]

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, 2016-12-15 19:23:55]

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

[commit-bot: I haz the power, 2016-12-15 19:23:57]

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

[michaeln, 2016-12-15 19:26:55]

https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota...
File storage/browser/quota/quota_manager.cc (right):

https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota...
storage/browser/quota/quota_manager.cc:89: return type == kStorageTypeTemporary
|| type == kStorageTypePersistent;
On 2016/12/15 18:57:08, altimin wrote:
> On 2016/12/15 18:49:31, brettw (ping after 24h) wrote:
> > Can you check with the original author, michaeln, about the changes to this
> > file? For this line in particular, I wouldn't have expected persistent
storage
> > to be allowed in incognito.
> 
> Sure, moved him from cc to reviewers.

lgtm

thnx, ok, yikes, i'll add some tests with kQuotaErrorNotSupported expectations

[michaeln, 2016-12-15 19:29:40]

> I wouldn't have expected persistent storage
> to be allowed in incognito.

is is so incognito is not (as) detectable

[altimin, 2016-12-15 20:51:19]

brettw@, there were failures in openh264, I disabled warnings for that
third-party library. Could you please sanity-check this?

[altimin, 2016-12-15 20:55:42]

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

[commit-bot: I haz the power, 2016-12-15 20:56:45]

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, 2016-12-15 21:36:32]

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

[commit-bot: I haz the power, 2016-12-15 21:36:33]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[brettw, 2016-12-15 22:10:45]

lgtm

[altimin, 2016-12-16 15:02:58]

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

[commit-bot: I haz the power, 2016-12-16 15:03: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, 2016-12-16 17:05:36]

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

[commit-bot: I haz the power, 2016-12-16 17:05:38]

Dry run: 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_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[altimin, 2016-12-16 18:59:58]

The CQ bit was checked by altimin@chromium.org

[altimin, 2016-12-16 18:59:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaeln@chromium.org, brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2581733002/#ps30001
(title: "Remove clang warnings only when using clang")

[commit-bot: I haz the power, 2016-12-16 19:00:42]

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, 2016-12-16 22:21:33]

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

[commit-bot: I haz the power, 2016-12-16 22:21:33]

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_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[altimin, 2016-12-16 23:42:18]

The CQ bit was checked by altimin@chromium.org

[commit-bot: I haz the power, 2016-12-16 23:43:07]

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, 2016-12-17 03:46:25]

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

[commit-bot: I haz the power, 2016-12-17 03:46:26]

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

[altimin, 2016-12-17 14:23:06]

The CQ bit was checked by altimin@chromium.org

[commit-bot: I haz the power, 2016-12-17 14:23:12]

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, 2016-12-17 18:51:08]

CQ is committing da patch.

Bot data: {"patchset_id": 30001, "attempt_start_ts": 1481984586754510,
"parent_rev": "18af521d307143cb692fb56772054f1c3f628b88", "commit_rev":
"09ba49867c0c97dc5a32b37c72b97a204ff51166"}

[commit-bot: I haz the power, 2016-12-17 18:51:36]

Description was changed from

==========
Fix tautological comparison and add clang flag
==========

to

==========
Fix tautological comparison and add clang flag

Review-Url: https://codereview.chromium.org/2581733002
==========

[commit-bot: I haz the power, 2016-12-17 18:51:37]

Committed patchset #3 (id:30001)

[commit-bot: I haz the power, 2016-12-17 18:54:20]

Description was changed from

==========
Fix tautological comparison and add clang flag

Review-Url: https://codereview.chromium.org/2581733002
==========

to

==========
Fix tautological comparison and add clang flag

Committed: https://crrev.com/1b142eb67aedcd40f07234e38174b5b0f9995dc5
Cr-Commit-Position: refs/heads/master@{#439346}
==========

[commit-bot: I haz the power, 2016-12-17 18:54:21]

Patchset 3 (id:??) landed as
https://crrev.com/1b142eb67aedcd40f07234e38174b5b0f9995dc5
Cr-Commit-Position: refs/heads/master@{#439346}

[jsbell, 2016-12-20 19:46:35]

jsbell@chromium.org changed reviewers:
+ jsbell@chromium.org

[jsbell, 2016-12-20 19:46:35]

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.