Use ContainsValue() instead of std::find() in chrome/browser/extensions

Use ContainsValue() instead of std::find() in chrome/browser/extensions

BUG=561800
Review-Url: https://codereview.chromium.org/2961033002
Cr-Commit-Position: refs/heads/master@{#484841}
Committed: https://chromium.googlesource.com/chromium/src/+/0ac673a26607b9b82c21b2f88987f396ec284c66

[Tripta, 2017-06-28 09:44:13]

Description was changed from

==========
Use ContainsValue() instead of std::find() in chrome/browser/extensions

BUG=561800
==========

to

==========
Use ContainsValue() instead of std::find() in chrome/browser/extensions

BUG=561800
==========

[Tripta, 2017-06-28 09:44:13]

tripta.g@samsung.com changed reviewers:
+ benwells@chromium.org

[Tripta, 2017-06-28 09:45:36]

Hi benwells,

Please review code changes for below files:

  chrome/browser/extensions/api/file_system/file_system_api.cc
  chrome/browser/extensions/api/gcm/gcm_apitest.cc
 
chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc
  chrome/browser/extensions/api/web_request/web_request_api_unittest.cc
  chrome/browser/extensions/extension_management.cc
  chrome/browser/extensions/extension_management_unittest.cc
  chrome/browser/extensions/extension_service.cc
  chrome/browser/extensions/updater/extension_updater.cc

[benwells, 2017-06-29 22:16:03]

benwells@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[benwells, 2017-06-29 22:16:04]

Sorry I was sheriffing the past couple of days and am just heading out on a
holiday (am at the airport now).

Punting review to Devlin.

[Devlin, 2017-06-30 01:52:00]

https://codereview.chromium.org/2961033002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/web_request/web_request_api_unittest.cc
(right):

https://codereview.chromium.org/2961033002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/web_request/web_request_api_unittest.cc:116:
return base::ContainsValue(collection, key);
Here, could we just replace all callers of Contains() with
base::ContainsValue()?

https://codereview.chromium.org/2961033002/diff/1/chrome/browser/extensions/u...
File chrome/browser/extensions/updater/extension_updater.cc (right):

https://codereview.chromium.org/2961033002/diff/1/chrome/browser/extensions/u...
chrome/browser/extensions/updater/extension_updater.cc:322: if
(downloader_->AddExtension(extension, request_id, fetch_priority))
nit: not this patch, but maybe combine this with the above?

if (!base::ContainsValue(...) &&
    downloader_->AddExtension(...)) {
  request...
}

[Tripta, 2017-07-03 05:34:44]

Have made changes. Please check.

https://codereview.chromium.org/2961033002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/web_request/web_request_api_unittest.cc
(right):

https://codereview.chromium.org/2961033002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/web_request/web_request_api_unittest.cc:116:
return base::ContainsValue(collection, key);
On 2017/06/30 01:51:59, Devlin (OOO through 5. July) wrote:
> Here, could we just replace all callers of Contains() with
> base::ContainsValue()?

Done.

https://codereview.chromium.org/2961033002/diff/1/chrome/browser/extensions/u...
File chrome/browser/extensions/updater/extension_updater.cc (right):

https://codereview.chromium.org/2961033002/diff/1/chrome/browser/extensions/u...
chrome/browser/extensions/updater/extension_updater.cc:322: if
(downloader_->AddExtension(extension, request_id, fetch_priority))
On 2017/06/30 01:52:00, Devlin (OOO through 5. July) wrote:
> nit: not this patch, but maybe combine this with the above?
> 
> if (!base::ContainsValue(...) &&
>     downloader_->AddExtension(...)) {
>   request...
> }

Done.

[Devlin, 2017-07-05 15:37:23]

One last issue

https://codereview.chromium.org/2961033002/diff/20001/chrome/browser/extensio...
File
chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc
(right):

https://codereview.chromium.org/2961033002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc:307:
languages.erase(it);
Does this compile?  The iterator erased here no longer exists...

I think this is probably a case where we don't want to use
base::ContainsValue().

[Tripta, 2017-07-06 14:20:56]

Patchset #4 (id:60001) has been deleted

[Tripta, 2017-07-06 14:21:23]

Patchset #3 (id:40001) has been deleted

[Tripta, 2017-07-06 14:29:04]

Hi Devlin, 	

My build setup does not compile file under chrome/browser/extensions. I keep
check of such cases. This one got missed.
Thanks.

https://codereview.chromium.org/2961033002/diff/20001/chrome/browser/extensio...
File
chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc
(right):

https://codereview.chromium.org/2961033002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc:307:
languages.erase(it);
On 2017/07/05 15:37:23, Devlin wrote:
> Does this compile?  The iterator erased here no longer exists...
> 
> I think this is probably a case where we don't want to use
> base::ContainsValue().

Done.

[Devlin, 2017-07-06 14:57:20]

lgtm, thanks for the cleanup!

[Tripta, 2017-07-07 04:51:23]

The CQ bit was checked by tripta.g@samsung.com

[commit-bot: I haz the power, 2017-07-07 04:51:29]

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-07-07 05:45:11]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1499403083567140,
"parent_rev": "b6c9801027065551008297762dacd1b6631e3aa1", "commit_rev":
"0ac673a26607b9b82c21b2f88987f396ec284c66"}

[commit-bot: I haz the power, 2017-07-07 05:45:21]

Description was changed from

==========
Use ContainsValue() instead of std::find() in chrome/browser/extensions

BUG=561800
==========

to

==========
Use ContainsValue() instead of std::find() in chrome/browser/extensions

BUG=561800

Review-Url: https://codereview.chromium.org/2961033002
Cr-Commit-Position: refs/heads/master@{#484841}
Committed:
https://chromium.googlesource.com/chromium/src/+/0ac673a26607b9b82c21b2f88987...
==========

[commit-bot: I haz the power, 2017-07-07 05:45:22]

Committed patchset #3 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/0ac673a26607b9b82c21b2f88987...