Misc. cleanup, primarily removing unused locals.

Misc. cleanup, primarily removing unused locals.

Also various other fixes, e.g. condensing code, converting DCHECK_LT(0, a) -> DCHECK_GT(a, 0) (and the like) for readability, inserting a few typecasts.

BUG=none
TEST=none

Committed: https://crrev.com/7bc277b97e9f5f3b470c4b6ccd9337ff73892159
Cr-Commit-Position: refs/heads/master@{#299362}

[Peter Kasting, 2014-10-08 19:21:55]

pkasting@chromium.org changed reviewers:
+ blundell@chromium.org, jam@chromium.org, rvargas@chromium.org,
sky@chromium.org

[Peter Kasting, 2014-10-08 19:21:56]

sky: chrome/, ui/ OWNERS
blundell: components/ OWNERS
jam: content/, ipc/ OWNERS
rvargas: net/ OWNERS

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/cache_uti...
File net/disk_cache/cache_util.cc (left):

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/cache_uti...
net/disk_cache/cache_util.cc:156: return implicit_cast<int32>(max_size);
This needs a static_cast, not an implicit_cast, to avoid compiler warnings.

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/simple/si...
File net/disk_cache/simple/simple_synchronous_entry.cc (right):

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/simple/si...
net/disk_cache/simple/simple_synchronous_entry.cc:566: int64 avail_so_far = 0;
The type changes and typecasts inserted in this file are to avoid various
currently-disabled MSVC compiler warnings.

https://codereview.chromium.org/637023002/diff/40001/net/spdy/spdy_frame_buil...
File net/spdy/spdy_frame_builder.cc (right):

https://codereview.chromium.org/637023002/diff/40001/net/spdy/spdy_frame_buil...
net/spdy/spdy_frame_builder.cc:171: if (version_ < SPDY4) {
This is just a consistency change -- all the DCHECKs in this file against SPDY
versions seemed to use < and > instead of <= and >=, so I just made these
conditionals do the same.

[grt (UTC plus 2), 2014-10-08 19:41:26]

chrome/installer/util/installer_state.cc lgtm

[rvargas (doing something else), 2014-10-08 20:01:14]

net LGTM

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/cache_uti...
File net/disk_cache/cache_util.cc (left):

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/cache_uti...
net/disk_cache/cache_util.cc:156: return implicit_cast<int32>(max_size);
On 2014/10/08 19:21:56, Peter Kasting wrote:
> This needs a static_cast, not an implicit_cast, to avoid compiler warnings.

Certainly!

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/simple/si...
File net/disk_cache/simple/simple_synchronous_entry.cc (right):

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/simple/si...
net/disk_cache/simple/simple_synchronous_entry.cc:108: DCHECK_GE(file_index, 0);
Thanks for fixing all of these.

[sky, 2014-10-08 22:28:58]

LGTM

[blundell, 2014-10-09 05:58:29]

//components LGTM

[Peter Kasting, 2014-10-13 19:52:00]

pkasting@chromium.org changed reviewers:
+ avi@chromium.org, tsepez@chromium.org
- jam@chromium.org

[Peter Kasting, 2014-10-13 19:52:01]

jam seems to be unresponsive, so:
avi: content/ OWNERS
tsepez: ipc/ OWNERS

[Avi (use Gerrit), 2014-10-13 20:04:14]

content stuff LGTM

[Tom Sepez, 2014-10-13 20:06:19]

ipc/ LGTM.

[Peter Kasting, 2014-10-13 20:12:54]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2014-10-13 20:14:49]

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

[commit-bot: I haz the power, 2014-10-13 20:58:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-13 20:59:56]

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

[pasko, 2015-12-09 18:40:42]

Description was changed from

==========
Misc. cleanup, primarily removing unused locals.

Also various other fixes, e.g. condensing code, converting DCHECK_LT(0, a) ->
DCHECK_GT(a, 0) (and the like) for readability, inserting a few typecasts.

BUG=none
TEST=none

Committed: https://crrev.com/7bc277b97e9f5f3b470c4b6ccd9337ff73892159
Cr-Commit-Position: refs/heads/master@{#299362}
==========

to

==========
Misc. cleanup, primarily removing unused locals.

Also various other fixes, e.g. condensing code, converting DCHECK_LT(0, a) ->
DCHECK_GT(a, 0) (and the like) for readability, inserting a few typecasts.

BUG=none
TEST=none

Committed: https://crrev.com/7bc277b97e9f5f3b470c4b6ccd9337ff73892159
Cr-Commit-Position: refs/heads/master@{#299362}
==========

[pasko, 2015-12-09 18:40:42]

pasko@chromium.org changed reviewers:
+ gavinp@chromium.org, ttuttle@chromium.org

[pasko, 2015-12-09 18:41:06]

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

[pasko, 2015-12-09 18:41:07]

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/simple/si...
File net/disk_cache/simple/simple_synchronous_entry.cc (right):

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/simple/si...
net/disk_cache/simple/simple_synchronous_entry.cc:438: int range_len =
base::saturated_cast<int>(found_range->length);
Is this actually right?

I am not very familiar with the sparse cache code, but it seems wrong to CHECK
the browser process on reading inconsistent data from disk without ability to
recover. Some bug in former versions of Chrome or a bit flip on disk can make us
a query of death.

hopefully range lengths are corrected earlier, +ttuttle, +gavinp on this

[Peter Kasting, 2015-12-09 22:07:23]

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/simple/si...
File net/disk_cache/simple/simple_synchronous_entry.cc (right):

https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/simple/si...
net/disk_cache/simple/simple_synchronous_entry.cc:438: int range_len =
base::saturated_cast<int>(found_range->length);
On 2015/12/09 18:41:06, pasko wrote:
> Is this actually right?
> 
> I am not very familiar with the sparse cache code, but it seems wrong to CHECK
> the browser process on reading inconsistent data from disk without ability to
> recover. Some bug in former versions of Chrome or a bit flip on disk can make
us
> a query of death.
> 
> hopefully range lengths are corrected earlier, +ttuttle, +gavinp on this

saturated_cast is not checked_cast.  This change does not change the function of
the code.

[pasko, 2015-12-10 09:50:16]

On 2015/12/09 22:07:23, Peter Kasting wrote:
>
https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/simple/si...
> File net/disk_cache/simple/simple_synchronous_entry.cc (right):
> 
>
https://codereview.chromium.org/637023002/diff/40001/net/disk_cache/simple/si...
> net/disk_cache/simple/simple_synchronous_entry.cc:438: int range_len =
> base::saturated_cast<int>(found_range->length);
> On 2015/12/09 18:41:06, pasko wrote:
> > Is this actually right?
> > 
> > I am not very familiar with the sparse cache code, but it seems wrong to
CHECK
> > the browser process on reading inconsistent data from disk without ability
to
> > recover. Some bug in former versions of Chrome or a bit flip on disk can
make
> us
> > a query of death.
> > 
> > hopefully range lengths are corrected earlier, +ttuttle, +gavinp on this
> 
> saturated_cast is not checked_cast.  This change does not change the function
of
> the code.

thanks. I just saw a CHECK there and did not dive into details of the template
instantiations. This one seems safe.