Rewriting patch logic without the comma operator.

Rewriting patch logic without the comma operator.

When running /analyze on all of Chrome there were three warnings about
using the comma operator in a tested expression. One was a genuine bug
and the other two were in UncompressAndPatchChromeArchive. Rewriting
to use nested if statements resolves these warnings (getting the count
to zero) and makes the code clearer.

Resolving the warning is not crucial, but it makes it easier to make a
zero-tolerance policy for that warning and thus avoid future bugs.
Plus, it makes the setup code easier to read.

The warnings are:

src\chrome\installer\setup\setup_main.cc(174) : warning C6319: Use of the comma-operator in a tested expression causes the left argument to be ignored when it has no side-effects.
src\chrome\installer\setup\setup_main.cc(176) : warning C6319: Use of the comma-operator in a tested expression causes the left argument to be ignored when it has no side-effects.

The bug that was found through this warning, which could eventually
have been very serious, was fixed here:
https://codereview.chromium.org/678193003

BUG=427616
Committed: https://crrev.com/944c28c5ee375cd1eec61e4530750e37c29f65be
Cr-Commit-Position: refs/heads/master@{#314389}

[brucedawson, 2015-02-03 18:43:29]

brucedawson@chromium.org changed reviewers:
+ grt@chromium.org

[brucedawson, 2015-02-03 18:44:00]

What do you think about this? Clearer?

The code builds but I'm not sure how to test it.

[grt (UTC plus 2), 2015-02-03 18:49:08]

lgtm with a formatting nit. thanks for taking care of this.

https://codereview.chromium.org/893703004/diff/1/chrome/installer/setup/setup...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/893703004/diff/1/chrome/installer/setup/setup...
chrome/installer/setup/setup_main.cc:179: IDS_INSTALL_UNCOMPRESSION_FAILED_BASE,
the indentation here should be something more like:
      installer_state.WriteInstallerResult(
          *install_status, IDS_INSTALL_UNCOMPRESSION_FAILED_BASE, NULL);

[brucedawson, 2015-02-03 19:02:22]

Any thoughts on testing? Are try-bots/canary sufficient? I'm not sure how to
practically exercise this.

https://codereview.chromium.org/893703004/diff/1/chrome/installer/setup/setup...
File chrome/installer/setup/setup_main.cc (right):

https://codereview.chromium.org/893703004/diff/1/chrome/installer/setup/setup...
chrome/installer/setup/setup_main.cc:179: IDS_INSTALL_UNCOMPRESSION_FAILED_BASE,
I was wondering how to best do that. Aligning all of the arguments pushed line
179 to 81 characters. I'll make that change.

[grt (UTC plus 2), 2015-02-03 19:06:16]

On 2015/02/03 19:02:22, brucedawson wrote:
> Any thoughts on testing? Are try-bots/canary sufficient? I'm not sure how to
> practically exercise this.

There is no automated test coverage for this corner of the installer (applying a
diff update), so Canary and dev channel are it for now. http://crbug.com/453807
is my hope for the future.

[brucedawson, 2015-02-03 19:16:37]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2015-02-03 19:18:04]

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

[commit-bot: I haz the power, 2015-02-03 19:54:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-03 19:55:54]

Patchset 2 (id:??) landed as
https://crrev.com/944c28c5ee375cd1eec61e4530750e37c29f65be
Cr-Commit-Position: refs/heads/master@{#314389}