ImageWriter: Add ShouldResetImageWriter helper

ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.
Add comments, refer back to the original crbug.com issue and change
set that resolved issue 352442.

Covered by existing tests:
  image_writer_utility_client_browsertest, image_writer_unittest

BUG=680928, 352442
Review-Url: https://codereview.chromium.org/2734773002
Cr-Commit-Position: refs/heads/master@{#458024}
Committed: https://chromium.googlesource.com/chromium/src/+/4342d3eacba11f513071dd4bb06b182b4f1245f3

[Noel Gordon, 2017-03-06 06:18:41]

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

[commit-bot: I haz the power, 2017-03-06 06:18:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Noel Gordon, 2017-03-06 06:22:39]

noel@chromium.org changed reviewers:
+ dcheng@chromium.org, haven@chromium.org

[Noel Gordon, 2017-03-06 06:22:40]

This came up on https://codereview.chromium.org/2663603002 where it was
suggested to add a helper.

haven@ could you review please.

[commit-bot: I haz the power, 2017-03-06 06:57:44]

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

[commit-bot: I haz the power, 2017-03-06 06:57:45]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-03-06 21:47:46]

https://codereview.chromium.org/2734773002/diff/1/chrome/utility/image_writer...
File chrome/utility/image_writer/image_writer_handler.cc (right):

https://codereview.chromium.org/2734773002/diff/1/chrome/utility/image_writer...
chrome/utility/image_writer/image_writer_handler.cc:123: if (!image_writer_)
Please link to the original bug and also comment here why it's important not to
reset if we're writing to the same image:

When writing and verifying the same image, it's important not to
close the file handles in between. Otherwise (insert reason from
original commit description)

[Noel Gordon, 2017-03-17 02:50:12]

Description was changed from

==========
ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.

BUG=680928,352442
==========

to

==========
ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.
Add comments, refer back to the original crbug.com issue and change
set that resolved issue 352442.

BUG=680928,352442
==========

[Noel Gordon, 2017-03-17 02:51:01]

noel@chromium.org changed reviewers:
+ sammc@chromium.org, tibell@chromium.org

[Noel Gordon, 2017-03-17 02:51:29]

PTAL

https://codereview.chromium.org/2734773002/diff/1/chrome/utility/image_writer...
File chrome/utility/image_writer/image_writer_handler.cc (right):

https://codereview.chromium.org/2734773002/diff/1/chrome/utility/image_writer...
chrome/utility/image_writer/image_writer_handler.cc:123: if (!image_writer_)
On 2017/03/06 21:47:46, dcheng wrote:
> Please link to the original bug and also comment here why it's important not
to
> reset if we're writing to the same image:
> 
> When writing and verifying the same image, it's important not to
> close the file handles in between. Otherwise (insert reason from
> original commit description)

Done.

[tibell, 2017-03-17 02:52:42]

lgtm

[Sam McNally, 2017-03-17 02:53:24]

lgtm

[dcheng, 2017-03-17 05:38:20]

LGTM, thanks for the cleanup

[Noel Gordon, 2017-03-17 05:43:27]

noel@chromium.org changed reviewers:
+ jochen@chromium.org

[Noel Gordon, 2017-03-17 05:43:28]

+jochen for OWNERS

[jochen (gone - plz use gerrit), 2017-03-17 14:12:03]

which test covers this code?

[Noel Gordon, 2017-03-18 00:22:34]

On 2017/03/17 14:12:03, jochen wrote:
> which test covers this code?

I added a note about the test: image_writer_utility_client_browsertest.

[Noel Gordon, 2017-03-18 00:25:39]

Description was changed from

==========
ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.
Add comments, refer back to the original crbug.com issue and change
set that resolved issue 352442.

BUG=680928,352442
==========

to

==========
ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.
Add comments, refer back to the original crbug.com issue and change
set that resolved issue 352442.

Covered by browser test: image_writer_utility_client_browsertest.

BUG=680928,352442
==========

[Noel Gordon, 2017-03-18 00:43:21]

Description was changed from

==========
ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.
Add comments, refer back to the original crbug.com issue and change
set that resolved issue 352442.

Covered by browser test: image_writer_utility_client_browsertest.

BUG=680928,352442
==========

to

==========
ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.
Add comments, refer back to the original crbug.com issue and change
set that resolved issue 352442.

Covered by tests:
  image_writer_utility_client_browsertest, image_writer_unittest

BUG=680928,352442
==========

[Noel Gordon, 2017-03-18 23:34:29]

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

[commit-bot: I haz the power, 2017-03-18 23:34:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-19 00:15:35]

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

[commit-bot: I haz the power, 2017-03-19 00:15:36]

Dry run: This issue passed the CQ dry run.

[Noel Gordon, 2017-03-19 01:14:49]

On 2017/03/18 00:22:34, noel gordon wrote:
> On 2017/03/17 14:12:03, jochen wrote:
> > which test covers this code?
> 
> I added a note about the test: image_writer_utility_client_browsertest.

+ image_writer_unittest

[Noel Gordon, 2017-03-19 01:15:19]

Description was changed from

==========
ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.
Add comments, refer back to the original crbug.com issue and change
set that resolved issue 352442.

Covered by tests:
  image_writer_utility_client_browsertest, image_writer_unittest

BUG=680928,352442
==========

to

==========
ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.
Add comments, refer back to the original crbug.com issue and change
set that resolved issue 352442.

Covered by existing tests:
  image_writer_utility_client_browsertest, image_writer_unittest

BUG=680928,352442
==========

[jochen (gone - plz use gerrit), 2017-03-20 07:13:13]

lgtm

[Noel Gordon, 2017-03-20 07:24:55]

The CQ bit was checked by noel@chromium.org

[commit-bot: I haz the power, 2017-03-20 07:25:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-20 08:08:36]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489994695492570,
"parent_rev": "2c8b6311374bd76e4371e2a28699a7ecef9cf968", "commit_rev":
"4342d3eacba11f513071dd4bb06b182b4f1245f3"}

[commit-bot: I haz the power, 2017-03-20 08:09:19]

Description was changed from

==========
ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.
Add comments, refer back to the original crbug.com issue and change
set that resolved issue 352442.

Covered by existing tests:
  image_writer_utility_client_browsertest, image_writer_unittest

BUG=680928,352442
==========

to

==========
ImageWriter: Add ShouldResetImageWriter helper

Move the code for resetting the image_writer_ into a common helper.
Add comments, refer back to the original crbug.com issue and change
set that resolved issue 352442.

Covered by existing tests:
  image_writer_utility_client_browsertest, image_writer_unittest

BUG=680928,352442

Review-Url: https://codereview.chromium.org/2734773002
Cr-Commit-Position: refs/heads/master@{#458024}
Committed:
https://chromium.googlesource.com/chromium/src/+/4342d3eacba11f513071dd4bb06b...
==========

[commit-bot: I haz the power, 2017-03-20 08:09:21]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4342d3eacba11f513071dd4bb06b...