Remove kuint32max.

Remove kuint32max.

BUG=138542, 488550
Committed: https://crrev.com/c027914abee85b34b8223ced83c42200ec894d1f
Cr-Commit-Position: refs/heads/master@{#363309}

[Avi (use Gerrit), 2015-12-03 20:43:00]

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

[commit-bot: I haz the power, 2015-12-03 20:45:32]

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

[commit-bot: I haz the power, 2015-12-03 21:51:41]

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

[commit-bot: I haz the power, 2015-12-03 21:51:42]

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

[Avi (use Gerrit), 2015-12-03 21:55:43]

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

[commit-bot: I haz the power, 2015-12-03 21:57:06]

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

[commit-bot: I haz the power, 2015-12-04 00:08:46]

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

[commit-bot: I haz the power, 2015-12-04 00:08:47]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build URL)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build URL)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)

[Avi (use Gerrit), 2015-12-04 00:14:44]

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

[commit-bot: I haz the power, 2015-12-04 00:18:55]

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

[commit-bot: I haz the power, 2015-12-04 02:27:04]

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

[commit-bot: I haz the power, 2015-12-04 02:27:05]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)

[Avi (use Gerrit), 2015-12-04 14:46:29]

avi@chromium.org changed reviewers:
+ cpu@chromium.org, dalecurtis@chromium.org, gab@chromium.org,
mark@chromium.org, mattm@chromium.org, pfeldman@chromium.org,
phajdan.jr@chromium.org, reillyg@chromium.org

[Avi (use Gerrit), 2015-12-04 14:46:30]

mark: base/ and ui/
reillyg: chrome/browser/media_galleries and extensions/
pfeldman: chrome/browser/devtools
phajdan.jr: chrome/browser/download
gab: chrome/installer
mattm: net/
dalecurtis: media/
cpu: sandbox/win/

[Mark Mentovai, 2015-12-04 14:49:39]

LGTM in base

[Mark Mentovai, 2015-12-04 14:50:34]

LGTM in ui too

[asanka, 2015-12-04 14:55:14]

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

[asanka, 2015-12-04 14:55:14]

chrome/browser/download/ LGTM

[asanka, 2015-12-04 15:12:35]

https://codereview.chromium.org/1492403002/diff/20001/net/dns/dns_response.h
File net/dns/dns_response.h (right):

https://codereview.chromium.org/1492403002/diff/20001/net/dns/dns_response.h#...
net/dns/dns_response.h:9: 
Nit: Why the extra whitespace?

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
File net/http/http_security_headers.cc (right):

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
net/http/http_security_headers.cc:21: static_assert(kMaxHSTSAgeSecs <=
UINT32_MAX, "kMaxHSTSAgeSecs too large");
Why not numeric_limits? Isn't it required to be constexpr in C++11, or is this
something that's not allowed yet? I don't see anything calling out the
constexprness of limits in chromium-cpp though.

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
net/http/http_security_headers.cc:44: if (!base::StringToInt64(s, &i) && i !=
INT64_MAX)
Why INT64_MAX instead of numeric_limits?

This doesn't need constexpr. Also StringToInt64 is documented to return the
"maximum value supported by the data type" upon overflow.

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
File net/http/http_security_headers.h (right):

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
net/http/http_security_headers.h:9: 
Here as well. The style guide doesn't require the extra space (
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes ).

[Avi (use Gerrit), 2015-12-04 15:19:28]

https://codereview.chromium.org/1492403002/diff/20001/net/dns/dns_response.h
File net/dns/dns_response.h (right):

https://codereview.chromium.org/1492403002/diff/20001/net/dns/dns_response.h#...
net/dns/dns_response.h:9: 
On 2015/12/04 15:12:34, asanka wrote:
> Nit: Why the extra whitespace?

If I don't, the presubmit fails.

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
File net/http/http_security_headers.cc (right):

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
net/http/http_security_headers.cc:21: static_assert(kMaxHSTSAgeSecs <=
UINT32_MAX, "kMaxHSTSAgeSecs too large");
On 2015/12/04 15:12:34, asanka wrote:
> Why not numeric_limits? Isn't it required to be constexpr in C++11, or is this
> something that's not allowed yet? I don't see anything calling out the
> constexprness of limits in chromium-cpp though.

There was some rumbling about whether we can rely on the constexprness of max(),
so I used the old-school contant here. I can switch if you want.

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
net/http/http_security_headers.cc:44: if (!base::StringToInt64(s, &i) && i !=
INT64_MAX)
On 2015/12/04 15:12:34, asanka wrote:
> Why INT64_MAX instead of numeric_limits?

Consistency with above. I'll switch if you want.

> Also StringToInt64 is documented to return the
> "maximum value supported by the data type" upon overflow.

This is a mechanical-ish change. I want to switch out the integer types and
upgrade the max constants. I don't want to make any real code changes, no matter
how correct they are.

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
File net/http/http_security_headers.h (right):

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
net/http/http_security_headers.h:9: 
On 2015/12/04 15:12:35, asanka wrote:
> Here as well. The style guide doesn't require the extra space (
> https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
).

Yes it does. It explicitly calls out C header files as needing a separate
section from the C++ header files. And the PRESUBMIT checks for this. See
_CheckIncludeOrderForScope in src/PRESUBMIT.py

[asanka, 2015-12-04 15:41:43]

/net/ LGTM. Could you also reference issue 488550 in the description?

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
File net/http/http_security_headers.cc (right):

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
net/http/http_security_headers.cc:44: if (!base::StringToInt64(s, &i) && i !=
INT64_MAX)
On 2015/12/04 15:19:28, Avi wrote:
> On 2015/12/04 15:12:34, asanka wrote:
> > Why INT64_MAX instead of numeric_limits?
> 
> Consistency with above. I'll switch if you want.

I'd prefer if we switch this instance and keep the above as-is if constexprness
is contentious. Otherwise the presence of an old-school constant would prevent
us from migrating the remainder of the code.

> > Also StringToInt64 is documented to return the
> > "maximum value supported by the data type" upon overflow.
> 
> This is a mechanical-ish change. I want to switch out the integer types and
> upgrade the max constants. I don't want to make any real code changes, no
matter
> how correct they are.

Yup. I was remarking that the API isn't specific about a named constant, just
that it's the maximum value for the numeric type.

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
File net/http/http_security_headers.h (right):

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
net/http/http_security_headers.h:9: 
On 2015/12/04 15:19:28, Avi wrote:
> On 2015/12/04 15:12:35, asanka wrote:
> > Here as well. The style guide doesn't require the extra space (
> >
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
> ).
> 
> Yes it does. It explicitly calls out C header files as needing a separate
> section from the C++ header files. And the PRESUBMIT checks for this. See
> _CheckIncludeOrderForScope in src/PRESUBMIT.py

Ah. I see.

[Avi (use Gerrit), 2015-12-04 15:47:43]

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

[commit-bot: I haz the power, 2015-12-04 15:48:15]

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

[Avi (use Gerrit), 2015-12-04 15:48:17]

Updated.

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
File net/http/http_security_headers.cc (right):

https://codereview.chromium.org/1492403002/diff/20001/net/http/http_security_...
net/http/http_security_headers.cc:44: if (!base::StringToInt64(s, &i) && i !=
INT64_MAX)
On 2015/12/04 15:41:43, asanka wrote:
> On 2015/12/04 15:19:28, Avi wrote:
> > On 2015/12/04 15:12:34, asanka wrote:
> > > Why INT64_MAX instead of numeric_limits?
> > 
> > Consistency with above. I'll switch if you want.
> 
> I'd prefer if we switch this instance and keep the above as-is if
constexprness
> is contentious. Otherwise the presence of an old-school constant would prevent
> us from migrating the remainder of the code.
> 
> > > Also StringToInt64 is documented to return the
> > > "maximum value supported by the data type" upon overflow.
> > 
> > This is a mechanical-ish change. I want to switch out the integer types and
> > upgrade the max constants. I don't want to make any real code changes, no
> matter
> > how correct they are.
> 
> Yup. I was remarking that the API isn't specific about a named constant, just
> that it's the maximum value for the numeric type.

Done.

[Reilly Grant (use Gerrit), 2015-12-04 16:08:42]

chrome/browser/media_galleries and extensions lgtm

[Avi (use Gerrit), 2015-12-04 18:00:53]

Description was changed from

==========
Remove kuint32max.

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

to

==========
Remove kuint32max.

BUG=138542, 488550
==========

[gab, 2015-12-04 18:58:29]

chrome/installer/ RS lgtm

[DaleCurtis, 2015-12-04 19:01:36]

media/ lgtm

[commit-bot: I haz the power, 2015-12-04 19:36:50]

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

[commit-bot: I haz the power, 2015-12-04 19:36:51]

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

[cpu_(ooo_6.6-7.5), 2015-12-04 19:55:27]

sandbox/win lgtm

[Avi (use Gerrit), 2015-12-04 20:36:15]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2015-12-04 20:36:16]

The patchset sent to the CQ was uploaded after l-g-t-m from asanka@chromium.org,
mark@chromium.org
Link to the patchset: https://codereview.chromium.org/1492403002/#ps40001
(title: "http security header file")

[commit-bot: I haz the power, 2015-12-04 20:37:23]

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

[commit-bot: I haz the power, 2015-12-04 22:39:02]

Description was changed from

==========
Remove kuint32max.

BUG=138542, 488550
==========

to

==========
Remove kuint32max.

BUG=138542, 488550
==========

[commit-bot: I haz the power, 2015-12-04 22:39:03]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-04 23:07:43]

Description was changed from

==========
Remove kuint32max.

BUG=138542, 488550
==========

to

==========
Remove kuint32max.

BUG=138542, 488550

Committed: https://crrev.com/c027914abee85b34b8223ced83c42200ec894d1f
Cr-Commit-Position: refs/heads/master@{#363309}
==========

[commit-bot: I haz the power, 2015-12-04 23:07:44]

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