Fixed all unused-variable Clang warnings on Windows.

Fixed all unused-variable Clang warnings on Windows.

Usually, fixed by removing the unused variables. Some variables had to
be guarded by #ifs. Some variables could be used instead.

Also removed unused test file sweep02_16b_mono_16KHz.raw.

BUG=505319
TBR=rpaquay@chromium.org

Committed: https://crrev.com/88867d310fc297151344fcf294d23554371b2644
Cr-Commit-Position: refs/heads/master@{#337767}

[Nico, 2015-07-02 17:34:25]

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

[Nico, 2015-07-02 17:34:26]

Super cool! It might be worth getting all the code changes in without the flag
change first to reduce risk. (In case the flag change breaks something in, say,
the official build config or something)

lgtm with comments for everything I can lg (base, chrome, ui)

https://codereview.chromium.org/1220133003/diff/1/components/autofill/core/br...
File components/autofill/core/browser/autofill_ie_toolbar_import_win_unittest.cc
(right):

https://codereview.chromium.org/1220133003/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_ie_toolbar_import_win_unittest.cc:36:
const wchar_t* const kSaltValue = L"salt";
nit: also change these all from `const wchar_* const foo = ` to `const wchar_t
foo[] = `

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
File sandbox/win/src/process_mitigations_test.cc (right):

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_test.cc:168: // static const int
MEM_EXECUTE_OPTION_ENABLE = 1;
?

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_test.cc:170: // static const int
MEM_EXECUTE_OPTION_ATL7_THUNK_EMULATION = 4;
?

https://codereview.chromium.org/1220133003/diff/1/ui/base/win/atl_module.h
File ui/base/win/atl_module.h (right):

https://codereview.chromium.org/1220133003/diff/1/ui/base/win/atl_module.h#ne...
ui/base/win/atl_module.h:31: static_cast<void>(module);  // Suppress unused
variable warnings.
ALLOW_UNUSED_LOCAL(module);

https://codereview.chromium.org/1220133003/diff/1/ui/gfx/win/dpi.cc
File ui/gfx/win/dpi.cc (right):

https://codereview.chromium.org/1220133003/diff/1/ui/gfx/win/dpi.cc#newcode17
ui/gfx/win/dpi.cc:17: // int kDefaultDPIY = 96;
?

[Matt Giuca, 2015-07-03 03:46:29]

Good suggestion, moved the flag changes to
https://codereview.chromium.org/1226573002/. Now we can land this before the
third-party changes go in.

https://codereview.chromium.org/1220133003/diff/1/components/autofill/core/br...
File components/autofill/core/browser/autofill_ie_toolbar_import_win_unittest.cc
(right):

https://codereview.chromium.org/1220133003/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_ie_toolbar_import_win_unittest.cc:36:
const wchar_t* const kSaltValue = L"salt";
On 2015/07/02 17:34:26, Nico wrote:
> nit: also change these all from `const wchar_* const foo = ` to `const wchar_t
> foo[] = `

Done.

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
File sandbox/win/src/process_mitigations_test.cc (right):

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_test.cc:168: // static const int
MEM_EXECUTE_OPTION_ENABLE = 1;
Well I thought that since this seems to represent a set of bit flags, we should
not remove the information about what each bit means from the code (in case
someone wants to use these bits in the future).

Should I just delete these?

https://codereview.chromium.org/1220133003/diff/1/ui/base/win/atl_module.h
File ui/base/win/atl_module.h (right):

https://codereview.chromium.org/1220133003/diff/1/ui/base/win/atl_module.h#ne...
ui/base/win/atl_module.h:31: static_cast<void>(module);  // Suppress unused
variable warnings.
On 2015/07/02 17:34:26, Nico wrote:
> ALLOW_UNUSED_LOCAL(module);

Done.

https://codereview.chromium.org/1220133003/diff/1/ui/gfx/win/dpi.cc
File ui/gfx/win/dpi.cc (right):

https://codereview.chromium.org/1220133003/diff/1/ui/gfx/win/dpi.cc#newcode17
ui/gfx/win/dpi.cc:17: // int kDefaultDPIY = 96;
Again, I didn't want to delete this information from the code in case it was
needed in the future. Maybe this should just be "int kDefaultDPI = 96", implying
that X and Y have the same DPI (the code below seems to be making that
assumption anyway, which is why it uses kDefaultDPIX but not kDefaultDPIY).

https://codereview.chromium.org/1220133003/diff/40001/chrome/browser/memory_d...
File chrome/browser/memory_details_win.cc (right):

https://codereview.chromium.org/1220133003/diff/40001/chrome/browser/memory_d...
chrome/browser/memory_details_win.cc:28: enum BrowserProcess {
This was my favourite one :)

That's not how you enum...

[Matt Giuca, 2015-07-03 03:54:45]

mgiuca@chromium.org changed reviewers:
+ cpu@chromium.org, keybuk@chromium.org

[Matt Giuca, 2015-07-03 03:54:45]

Nico: PTAL at patch sets 3 and 4 (there are a lot more files touched now that
I've compiled it on GYP).

keybuk: Please review changes in device/bluetooth.

cpu: Please review changes in everywhere Nico hasn't reviewed (do you have the
extension that shows light green for lg and dark green for not?).

[Nico, 2015-07-03 04:11:15]

still lgtm

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
File sandbox/win/src/process_mitigations_test.cc (right):

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_test.cc:168: // static const int
MEM_EXECUTE_OPTION_ENABLE = 1;
On 2015/07/03 03:46:29, Matt Giuca wrote:
> Well I thought that since this seems to represent a set of bit flags, we
should
> not remove the information about what each bit means from the code (in case
> someone wants to use these bits in the future).
> 
> Should I just delete these?

As-is, it looks like code that was left in accidentally. If you want to keep
them, I'd make the comments look less like code. (I'd just delete them here, but
maybe a sandbox/OWNER sees it differently)

[Matt Giuca, 2015-07-03 04:33:44]

mgiuca@chromium.org changed reviewers:
+ jschuh@chromium.org

[Matt Giuca, 2015-07-03 04:33:45]

+jschuh for a quick question about sandbox/win/src/process_mitigations_test.cc.

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
File sandbox/win/src/process_mitigations_test.cc (right):

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_test.cc:168: // static const int
MEM_EXECUTE_OPTION_ENABLE = 1;
jschuh: Can I get your quick opinion on this. You added these constants in
https://codereview.chromium.org/10690058 (and even then, two of them were
unused).

Can I just delete these two altogether, or should I leave them in a comment of
some form?

[jschuh, 2015-07-06 22:16:18]

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
File sandbox/win/src/process_mitigations_test.cc (right):

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_test.cc:170: // static const int
MEM_EXECUTE_OPTION_ATL7_THUNK_EMULATION = 4;
Yeah, looks like dead constants. lgtm on removing them

[Matt Giuca, 2015-07-07 04:52:17]

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
File sandbox/win/src/process_mitigations_test.cc (right):

https://codereview.chromium.org/1220133003/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_test.cc:170: // static const int
MEM_EXECUTE_OPTION_ATL7_THUNK_EMULATION = 4;
Well they were dead the moment they were added, so I was wondering if they were
there for future purposes, but sure, I'll just remove them.

[cpu_(ooo_6.6-7.5), 2015-07-07 17:39:24]

lgtm

go ahead and CQ this.

https://codereview.chromium.org/1220133003/diff/100001/chrome/browser/install...
File chrome/browser/install_verification/win/imported_module_verification.cc
(right):

https://codereview.chromium.org/1220133003/diff/100001/chrome/browser/install...
chrome/browser/install_verification/win/imported_module_verification.cc:83: //
See crbug.com/316274.
I see no reason for this... looking for somebody to own the bug.

https://codereview.chromium.org/1220133003/diff/100001/chrome/browser/memory_...
File chrome/browser/memory_details_win.cc (right):

https://codereview.chromium.org/1220133003/diff/100001/chrome/browser/memory_...
chrome/browser/memory_details_win.cc:38: };
woa!

https://codereview.chromium.org/1220133003/diff/100001/remoting/host/remoting...
File remoting/host/remoting_me2me_host.cc (right):

https://codereview.chromium.org/1220133003/diff/100001/remoting/host/remoting...
remoting/host/remoting_me2me_host.cc:491: char buf[4096];
stack allocations this big are bad news. I basically forces a page fault and
increases the stack watermark forever.

All this code around here looks pretty weird, I wonder if REMOTING_MULTI_PROCESS
is on or off ...

[Matt Giuca, 2015-07-08 02:33:36]

mgiuca@chromium.org changed reviewers:
+ rpaquay@chromium.org

[Matt Giuca, 2015-07-08 02:33:37]

+rpaquay for device/bluetooth.

[Nico, 2015-07-08 02:38:23]

On 2015/07/08 02:33:37, Matt Giuca wrote:
> +rpaquay for device/bluetooth.

(I think you can tbr device/; cpu and I both looked at it and cpu is toplevel
owner. if the device/bluetooth change isn't optimal it can be fixed up
post-review – it's only going to be style stuff as the patch is
behavior-preserving)

[Matt Giuca, 2015-07-08 02:51:28]

https://codereview.chromium.org/1220133003/diff/100001/chrome/browser/install...
File chrome/browser/install_verification/win/imported_module_verification.cc
(right):

https://codereview.chromium.org/1220133003/diff/100001/chrome/browser/install...
chrome/browser/install_verification/win/imported_module_verification.cc:83: //
See crbug.com/316274.
On 2015/07/07 17:39:24, cpu wrote:
> I see no reason for this... looking for somebody to own the bug.

Acknowledged.

https://codereview.chromium.org/1220133003/diff/100001/chrome/browser/memory_...
File chrome/browser/memory_details_win.cc (right):

https://codereview.chromium.org/1220133003/diff/100001/chrome/browser/memory_...
chrome/browser/memory_details_win.cc:38: };
I know, this was a highlight :)

https://codereview.chromium.org/1220133003/diff/100001/remoting/host/remoting...
File remoting/host/remoting_me2me_host.cc (right):

https://codereview.chromium.org/1220133003/diff/100001/remoting/host/remoting...
remoting/host/remoting_me2me_host.cc:491: char buf[4096];
According to remoting/remoting_options.gypi, it is 1 on Windows, 0 elsewhere.
(So on Windows we had an unused variable that we were using on the other
platforms.)

Surely there is an abstraction of fread in base somewhere? (All I could find is
base::ReadFileToString, which takes a path and doesn't work for stdin.)

At least we can heap-allocate this. I'll do it in a follow-up.

[Matt Giuca, 2015-07-08 02:52:09]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2015-07-08 02:52:10]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org,
jschuh@chromium.org
Link to the patchset: https://codereview.chromium.org/1220133003/#ps100001
(title: "Rebase.")

[Matt Giuca, 2015-07-08 02:52:25]

On 2015/07/08 02:38:23, Nico wrote:
> (I think you can tbr device/; cpu and I both looked at it and cpu is toplevel
> owner. if the device/bluetooth change isn't optimal it can be fixed up
> post-review – it's only going to be style stuff as the patch is
> behavior-preserving)

OK done.

[commit-bot: I haz the power, 2015-07-08 02:53:09]

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

[commit-bot: I haz the power, 2015-07-08 04:08:35]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-07-08 04:09:32]

Patchset 6 (id:??) landed as
https://crrev.com/88867d310fc297151344fcf294d23554371b2644
Cr-Commit-Position: refs/heads/master@{#337767}