Remove kuint16max.

Remove kuint16max.

BUG=138542
Committed: https://crrev.com/0d94fda8c494d7a6c27d065d782f42e99d834f46
Cr-Commit-Position: refs/heads/master@{#362431}

Committed: https://crrev.com/6516805e8c69ac65bcb7d52a2a208be97a7d2a50
Cr-Commit-Position: refs/heads/master@{#362472}

[Avi (use Gerrit), 2015-11-24 20:54:21]

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

[commit-bot: I haz the power, 2015-11-24 20:55:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475803002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475803002/1

[commit-bot: I haz the power, 2015-11-24 21:29:31]

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

[commit-bot: I haz the power, 2015-11-24 21:29:31]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)

[Avi (use Gerrit), 2015-11-24 21:39:55]

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

[commit-bot: I haz the power, 2015-11-24 21:41:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475803002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475803002/20001

[Avi (use Gerrit), 2015-11-24 21:42:48]

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

[commit-bot: I haz the power, 2015-11-24 21:44:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475803002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475803002/40001

[Avi (use Gerrit), 2015-11-24 22:25:07]

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

[commit-bot: I haz the power, 2015-11-24 22:25:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475803002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475803002/60001

[commit-bot: I haz the power, 2015-11-24 23:23:28]

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

[commit-bot: I haz the power, 2015-11-24 23:23:29]

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

[Avi (use Gerrit), 2015-11-25 15:18:22]

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

[commit-bot: I haz the power, 2015-11-25 15:18:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475803002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475803002/80001

[Avi (use Gerrit), 2015-11-25 16:36:47]

avi@chromium.org changed reviewers:
+ bartfab@chromium.org, cpu@chromium.org, mark@chromium.org,
rsleevi@chromium.org, sergeyu@chromium.org, sky@chromium.org, treib@chromium.org

[Avi (use Gerrit), 2015-11-25 16:36:48]

treib: extensions/
mark: base/
rsleevi: net/
sergeyu: jingle/
bartfab: components/policy/
cpu: sandbox/win/

[Marc Treib, 2015-11-25 16:39:16]

extensions/ lgtm

[Mark Mentovai, 2015-11-25 16:39:59]

LGTM

[commit-bot: I haz the power, 2015-11-25 17:41:35]

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

[commit-bot: I haz the power, 2015-11-25 17:41:36]

Dry run: This issue passed the CQ dry run.

[sky, 2015-11-25 17:49:19]

You didn't list what files you wanted me to review, but I'm assuming it's
chrome. LGTM

https://codereview.chromium.org/1475803002/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/webstore_installer.cc (right):

https://codereview.chromium.org/1475803002/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/webstore_installer.cc:126: std::string random_number =
base::Uint64ToString(
This code is weird. It's calling uint64tostring with a uint16. Why not
UintToString?

[bartfab (slow), 2015-11-25 17:51:03]

components/policy/core/browser/url_blacklist_manager.cc

[Avi (use Gerrit), 2015-11-25 18:08:35]

https://codereview.chromium.org/1475803002/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/webstore_installer.cc (right):

https://codereview.chromium.org/1475803002/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/webstore_installer.cc:126: std::string random_number =
base::Uint64ToString(
On 2015/11/25 17:49:19, sky wrote:
> This code is weird. It's calling uint64tostring with a uint16. Why not
> UintToString?

I'm trying to keep these big CLs staying strictly mechanical. That's a valid
question, but one I don't have an answer to.

[sky, 2015-11-25 18:21:37]

On 2015/11/25 18:08:35, Avi wrote:
>
https://codereview.chromium.org/1475803002/diff/80001/chrome/browser/extensio...
> File chrome/browser/extensions/webstore_installer.cc (right):
> 
>
https://codereview.chromium.org/1475803002/diff/80001/chrome/browser/extensio...
> chrome/browser/extensions/webstore_installer.cc:126: std::string random_number
=
> base::Uint64ToString(
> On 2015/11/25 17:49:19, sky wrote:
> > This code is weird. It's calling uint64tostring with a uint16. Why not
> > UintToString?
> 
> I'm trying to keep these big CLs staying strictly mechanical. That's a valid
> question, but one I don't have an answer to.

Understood. That's why I LGTMd it.

[bartfab (slow), 2015-11-25 18:26:19]

components/policy/core/browser/url_blacklist_manager.cc LGTM

[Ryan Sleevi, 2015-11-25 18:32:17]

I assume that the issues regarding C++11 and constexpr are worked out, such that
this isn't causing compiler pessimissm? I seem to recall discussion on
chromium-dev regarding this (vs, say, the UINT16_MAX from stdint.h)

LGTM, although I'm hoping you looked into the background on this :)

[Avi (use Gerrit), 2015-11-25 18:37:10]

On 2015/11/25 18:32:17, Ryan Sleevi wrote:
> I assume that the issues regarding C++11 and constexpr are worked out, such
that
> this isn't causing compiler pessimissm? I seem to recall discussion on
> chromium-dev regarding this (vs, say, the UINT16_MAX from stdint.h)
> 
> LGTM, although I'm hoping you looked into the background on this :)

You're referring to
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/Bvd9QU5pY9Q/hiCgg...
. No, I haven't looked into the performance implications here. I am careful to
not introduce static initializers, but I've been using std::numeric_limits.

If this regresses benchmarks I'll revert, but people tended to like
std::numeric_limits.

[Sergey Ulanov, 2015-11-25 19:39:57]

lgtm

[Avi (use Gerrit), 2015-11-25 21:30:00]

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

[commit-bot: I haz the power, 2015-11-25 21:34:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475803002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475803002/100001

[commit-bot: I haz the power, 2015-11-25 23:39:47]

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

[commit-bot: I haz the power, 2015-11-25 23:39:48]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)

[cpu_(ooo_6.6-7.5), 2015-11-30 18:33:06]

https://codereview.chromium.org/1475803002/diff/100001/sandbox/win/src/sandbo...
File sandbox/win/src/sandbox_nt_util.cc (right):

https://codereview.chromium.org/1475803002/diff/100001/sandbox/win/src/sandbo...
sandbox/win/src/sandbox_nt_util.cc:534:
DCHECK_NT(std::numeric_limits<uint16_t>::max() > size_bytes);
I don't know about this. If this ever translates to a call, or fetching a
constant from the crt we are in trouble because code in this file can run waaay
before we hit main, in fact kernel32.dll might not even be fully ready.

if you remove the std::numeric_litmits<uint16> calls then lgtm, if you want to
keep this I would need to understand how does this behave both on release and
debug and both with the crt as a dll.

https://codereview.chromium.org/1475803002/diff/100001/sandbox/win/src/sandbo...
sandbox/win/src/sandbox_nt_util.cc:612: file_info->FileNameLength >
std::numeric_limits<uint16_t>::max())
same here.

[Mark Mentovai, 2015-11-30 18:38:34]

https://codereview.chromium.org/1475803002/diff/100001/sandbox/win/src/sandbo...
File sandbox/win/src/sandbox_nt_util.cc (right):

https://codereview.chromium.org/1475803002/diff/100001/sandbox/win/src/sandbo...
sandbox/win/src/sandbox_nt_util.cc:534:
DCHECK_NT(std::numeric_limits<uint16_t>::max() > size_bytes);
cpu wrote:
> I don't know about this. If this ever translates to a call, or fetching a
> constant from the crt we are in trouble because code in this file can run
waaay
> before we hit main, in fact kernel32.dll might not even be fully ready.
> 
> if you remove the std::numeric_litmits<uint16> calls then lgtm, if you want to
> keep this I would need to understand how does this behave both on release and
> debug and both with the crt as a dll.

std::numeric_limits<>::max() is constexpr in the C++11 library, but that’s
probably not helpful until MSVS 2015.

This was last discussed in
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/Bvd9QU5pY9Q

[Avi (use Gerrit), 2015-11-30 22:12:24]

https://codereview.chromium.org/1475803002/diff/100001/sandbox/win/src/sandbo...
File sandbox/win/src/sandbox_nt_util.cc (right):

https://codereview.chromium.org/1475803002/diff/100001/sandbox/win/src/sandbo...
sandbox/win/src/sandbox_nt_util.cc:534:
DCHECK_NT(std::numeric_limits<uint16_t>::max() > size_bytes);
On 2015/11/30 18:38:34, Mark Mentovai wrote:
> cpu wrote:
> > I don't know about this. If this ever translates to a call, or fetching a
> > constant from the crt we are in trouble because code in this file can run
> waaay
> > before we hit main, in fact kernel32.dll might not even be fully ready.
> > 
> > if you remove the std::numeric_litmits<uint16> calls then lgtm, if you want
to
> > keep this I would need to understand how does this behave both on release
and
> > debug and both with the crt as a dll.
> 
> std::numeric_limits<>::max() is constexpr in the C++11 library, but that’s
> probably not helpful until MSVS 2015.
> 
> This was last discussed in
> https://groups.google.com/a/chromium.org/d/topic/chromium-dev/Bvd9QU5pY9Q

I can switch to UINT16_MAX if that would alleviate the concerns.

[cpu_(ooo_6.6-7.5), 2015-11-30 23:46:33]

yeah lets switch to that other macro.

[Avi (use Gerrit), 2015-12-01 14:21:33]

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

[commit-bot: I haz the power, 2015-12-01 14:22:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475803002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475803002/120001

[Avi (use Gerrit), 2015-12-01 15:45:13]

Switched.

[mmenke, 2015-12-01 15:45:44]

Description was changed from

==========
Remove kuint16max.

BUG=138542
==========

to

==========
Remove kuint16max.

BUG=138542
==========

[Avi (use Gerrit), 2015-12-01 16:20:17]

The CQ bit was unchecked by avi@chromium.org

[Avi (use Gerrit), 2015-12-01 16:20:53]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2015-12-01 16:20:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
bartfab@chromium.org, sky@chromium.org, rsleevi@chromium.org,
sergeyu@chromium.org, mark@chromium.org, treib@chromium.org, cpu@chromium.org
Link to the patchset: https://codereview.chromium.org/1475803002/#ps120001
(title: "UINT16_MAX")

[commit-bot: I haz the power, 2015-12-01 16:21:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475803002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475803002/120001

[commit-bot: I haz the power, 2015-12-01 16:35:27]

Description was changed from

==========
Remove kuint16max.

BUG=138542
==========

to

==========
Remove kuint16max.

BUG=138542
==========

[commit-bot: I haz the power, 2015-12-01 16:35:28]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-12-01 16:36:54]

Description was changed from

==========
Remove kuint16max.

BUG=138542
==========

to

==========
Remove kuint16max.

BUG=138542

Committed: https://crrev.com/0d94fda8c494d7a6c27d065d782f42e99d834f46
Cr-Commit-Position: refs/heads/master@{#362431}
==========

[commit-bot: I haz the power, 2015-12-01 16:36:56]

Patchset 7 (id:??) landed as
https://crrev.com/0d94fda8c494d7a6c27d065d782f42e99d834f46
Cr-Commit-Position: refs/heads/master@{#362431}

[Avi (use Gerrit), 2015-12-01 17:19:28]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/1485853003/ by avi@chromium.org.

The reason for reverting is: missed one :(.

[Avi (use Gerrit), 2015-12-01 17:26:16]

Description was changed from

==========
Remove kuint16max.

BUG=138542

Committed: https://crrev.com/0d94fda8c494d7a6c27d065d782f42e99d834f46
Cr-Commit-Position: refs/heads/master@{#362431}
==========

to

==========
Remove kuint16max.

BUG=138542

Committed: https://crrev.com/0d94fda8c494d7a6c27d065d782f42e99d834f46
Cr-Commit-Position: refs/heads/master@{#362431}
==========

[Avi (use Gerrit), 2015-12-01 17:35:34]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2015-12-01 17:35:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
bartfab@chromium.org, sky@chromium.org, rsleevi@chromium.org,
sergeyu@chromium.org, cpu@chromium.org, mark@chromium.org, treib@chromium.org
Link to the patchset: https://codereview.chromium.org/1475803002/#ps140001
(title: "cloud print")

[commit-bot: I haz the power, 2015-12-01 17:35:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1475803002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1475803002/140001

[commit-bot: I haz the power, 2015-12-01 19:27:19]

Description was changed from

==========
Remove kuint16max.

BUG=138542

Committed: https://crrev.com/0d94fda8c494d7a6c27d065d782f42e99d834f46
Cr-Commit-Position: refs/heads/master@{#362431}
==========

to

==========
Remove kuint16max.

BUG=138542

Committed: https://crrev.com/0d94fda8c494d7a6c27d065d782f42e99d834f46
Cr-Commit-Position: refs/heads/master@{#362431}
==========

[commit-bot: I haz the power, 2015-12-01 19:27:20]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-12-01 19:35:43]

Description was changed from

==========
Remove kuint16max.

BUG=138542

Committed: https://crrev.com/0d94fda8c494d7a6c27d065d782f42e99d834f46
Cr-Commit-Position: refs/heads/master@{#362431}
==========

to

==========
Remove kuint16max.

BUG=138542

Committed: https://crrev.com/0d94fda8c494d7a6c27d065d782f42e99d834f46
Cr-Commit-Position: refs/heads/master@{#362431}

Committed: https://crrev.com/6516805e8c69ac65bcb7d52a2a208be97a7d2a50
Cr-Commit-Position: refs/heads/master@{#362472}
==========

[commit-bot: I haz the power, 2015-12-01 19:35:46]

Patchset 8 (id:??) landed as
https://crrev.com/6516805e8c69ac65bcb7d52a2a208be97a7d2a50
Cr-Commit-Position: refs/heads/master@{#362472}