Replace ScopedVector with vector<scoped_ptr> in BitmapFetcherService.

Replace ScopedVector with vector<scoped_ptr> in BitmapFetcherService.

R=petewil@chromium.org, thakis@chromium.org
BUG=554289
Committed: https://crrev.com/dde4d8c4b2aa0906b4c92922f827ce5d3e234bf1
Cr-Commit-Position: refs/heads/master@{#359019}

[danakj, 2015-11-11 01:12:00]

Description was changed from

==========
Replace ScopedVector with vector<scoped_ptr> in BitmapFetcherService.

R=groby@chromium.org, thakis@chromium.org
BUG=554289
==========

to

==========
Replace ScopedVector with vector<scoped_ptr> in BitmapFetcherService.

R=petewil@chromium.org, thakis@chromium.org
BUG=554289
==========

[danakj, 2015-11-11 01:12:00]

danakj@chromium.org changed reviewers:
+ petewil@chromium.org
- groby@chromium.org

[danakj, 2015-11-11 01:12:27]

A test for the bots to make sure this works before we announce.

[danakj, 2015-11-11 01:13:10]

And I missed the unittest file in the first upload. Fixed.

[danakj, 2015-11-11 01:13:59]

Patchset #1 (id:1) has been deleted

[Nico, 2015-11-11 01:18:07]

lgtm, thanks!

[Pete Williamson, 2015-11-11 01:25:42]

lgtm

[Nico, 2015-11-11 01:28:23]

https://codereview.chromium.org/1433013004/diff/20001/chrome/browser/bitmap_f...
File chrome/browser/bitmap_fetcher/bitmap_fetcher_service_unittest.cc (right):

https://codereview.chromium.org/1433013004/diff/20001/chrome/browser/bitmap_f...
chrome/browser/bitmap_fetcher/bitmap_fetcher_service_unittest.cc:70: return
service_->active_fetchers_;
oh i guess this needs changing too?

[danakj, 2015-11-11 01:29:08]

https://codereview.chromium.org/1433013004/diff/20001/chrome/browser/bitmap_f...
File chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc (right):

https://codereview.chromium.org/1433013004/diff/20001/chrome/browser/bitmap_f...
chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc:157:
std::find(active_fetchers_.begin(), active_fetchers_.end(), fetcher);
find() won't work with unique_ptr cuz it doesn't have operator==(T*), you'd have
to compare to another unique_ptr. We should remove that from scoped_ptr, so I
won't add a use of it here, and reverted back to a for loop (I could do find_if
but that's pretty verbose).

https://codereview.chromium.org/1433013004/diff/20001/chrome/browser/bitmap_f...
File chrome/browser/bitmap_fetcher/bitmap_fetcher_service_unittest.cc (right):

https://codereview.chromium.org/1433013004/diff/20001/chrome/browser/bitmap_f...
chrome/browser/bitmap_fetcher/bitmap_fetcher_service_unittest.cc:70: return
service_->active_fetchers_;
On 2015/11/11 01:28:23, Nico wrote:
> oh i guess this needs changing too?

ah yes thanks.

[danakj, 2015-11-11 01:32:51]

Patchset #2 (id:40001) has been deleted

[danakj, 2015-11-11 01:34:33]

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

[commit-bot: I haz the power, 2015-11-11 01:35:28]

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

[Nico, 2015-11-11 01:35:53]

still lgtm, but:

https://codereview.chromium.org/1433013004/diff/60001/chrome/browser/bitmap_f...
File chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc (right):

https://codereview.chromium.org/1433013004/diff/60001/chrome/browser/bitmap_f...
chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc:156: auto it =
active_fetchers_.begin();
auto here is cool, but the rest of the loop change seems like a sideways change
– i'd keep it like it currently is if find() won't work i think

[Nico, 2015-11-11 01:42:28]

The CQ bit was unchecked by thakis@chromium.org

[Nico, 2015-11-11 01:42:36]

The CQ bit was checked by thakis@chromium.org

[Nico, 2015-11-11 01:42:37]

The patchset sent to the CQ was uploaded after l-g-t-m from petewil@chromium.org
Link to the patchset: https://codereview.chromium.org/1433013004/#ps60001
(title: "scopedvector-bitmapfetcher: no-find-fix-unittest")

[commit-bot: I haz the power, 2015-11-11 01:43:18]

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

[danakj, 2015-11-11 01:45:57]

https://codereview.chromium.org/1433013004/diff/60001/chrome/browser/bitmap_f...
File chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc (right):

https://codereview.chromium.org/1433013004/diff/60001/chrome/browser/bitmap_f...
chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc:156: auto it =
active_fetchers_.begin();
On 2015/11/11 01:35:53, Nico wrote:
> auto here is cool, but the rest of the loop change seems like a sideways
change
> – i'd keep it like it currently is if find() won't work i think

(we discussed this in person and nico was basically fine with it and hit the cq
for me. implementing this closer to find seems nice and now not-in-the-list isnt
handled in release which the styleguide asks us to not do)

[commit-bot: I haz the power, 2015-11-11 02:22:22]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2015-11-11 02:23:19]

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