Use ContainsValue() instead of std::find() in ios/

Use ContainsValue() instead of std::find() in ios/

BUG=561800
Review-Url: https://codereview.chromium.org/2945803002
Cr-Commit-Position: refs/heads/master@{#483300}
Committed: https://chromium.googlesource.com/chromium/src/+/33e703e1de0b77da838981dae4639e84c72aaea1

[Tripta, 2017-06-19 10:29:13]

Description was changed from

==========
Use ContainsValue() instead of std::find() in ios/

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

to

==========
Use ContainsValue() instead of std::find() in ios/

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

[Tripta, 2017-06-19 10:29:14]

tripta.g@samsung.com changed reviewers:
+ rohitrao@chromium.org, rohitrao@google.com

[Tripta, 2017-06-19 10:29:54]

Hi Rohit,

Please review the code changes for below files:

  ios/chrome/browser/bookmarks/bookmarks_utils.cc
  ios/chrome/browser/passwords/password_generation_agent.mm
  ios/chrome/browser/payments/payment_request.mm
  ios/chrome/browser/reading_list/url_downloader.cc
  ios/chrome/browser/reading_list/url_downloader_unittest.mm
  ios/chrome/browser/ui/bookmarks/bookmark_collection_view.mm
  ios/chrome/browser/voice/speech_input_locale_config_impl.mm
  ios/web/webui/url_data_manager_ios.cc
  ios/web/webui/url_data_manager_ios_backend.mm

Regards
Tripta

[sdefresne, 2017-06-23 14:31:21]

drive-by

https://codereview.chromium.org/2945803002/diff/1/ios/chrome/browser/bookmark...
File ios/chrome/browser/bookmarks/bookmarks_utils.cc (right):

https://codereview.chromium.org/2945803002/diff/1/ios/chrome/browser/bookmark...
ios/chrome/browser/bookmarks/bookmarks_utils.cc:76: if
(base::ContainsValue(primary_nodes, node)) {
This can be changed to the following:

  return base::ContainsValue(primary_nodes, node);

https://codereview.chromium.org/2945803002/diff/1/ios/chrome/browser/password...
File ios/chrome/browser/passwords/password_generation_agent.mm (right):

https://codereview.chromium.org/2945803002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/password_generation_agent.mm:47: bool
VectorContainsURL(const std::vector<GURL>& urls, const GURL& url) {
There is only one call site for VectorContainsURL (in the current file). Maybe
inline the base::ContainsValue() and remove the function (as it is just a
specialized instantiation of base::ContainsValue).

https://codereview.chromium.org/2945803002/diff/1/ios/web/webui/url_data_mana...
File ios/web/webui/url_data_manager_ios_backend.mm (right):

https://codereview.chromium.org/2945803002/diff/1/ios/web/webui/url_data_mana...
ios/web/webui/url_data_manager_ios_backend.mm:57: bool SchemeIsInSchemes(const
std::string& scheme,
ditto, just inline base::ContainsValue in the one callsite

[Tripta, 2017-06-27 06:39:18]

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

[commit-bot: I haz the power, 2017-06-27 06:39:26]

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-06-27 06:39:27]

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

[commit-bot: I haz the power, 2017-06-27 06:39:28]

No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[Tripta, 2017-06-27 06:41:29]

Hi sdefresne,

Have made the code changes. Please check.

Regards
Tripta

https://codereview.chromium.org/2945803002/diff/1/ios/chrome/browser/bookmark...
File ios/chrome/browser/bookmarks/bookmarks_utils.cc (right):

https://codereview.chromium.org/2945803002/diff/1/ios/chrome/browser/bookmark...
ios/chrome/browser/bookmarks/bookmarks_utils.cc:76: if
(base::ContainsValue(primary_nodes, node)) {
On 2017/06/23 14:31:21, sdefresne wrote:
> This can be changed to the following:
> 
>   return base::ContainsValue(primary_nodes, node);

Done.

https://codereview.chromium.org/2945803002/diff/1/ios/chrome/browser/password...
File ios/chrome/browser/passwords/password_generation_agent.mm (right):

https://codereview.chromium.org/2945803002/diff/1/ios/chrome/browser/password...
ios/chrome/browser/passwords/password_generation_agent.mm:47: bool
VectorContainsURL(const std::vector<GURL>& urls, const GURL& url) {
On 2017/06/23 14:31:21, sdefresne wrote:
> There is only one call site for VectorContainsURL (in the current file). Maybe
> inline the base::ContainsValue() and remove the function (as it is just a
> specialized instantiation of base::ContainsValue).

Done.

https://codereview.chromium.org/2945803002/diff/1/ios/web/webui/url_data_mana...
File ios/web/webui/url_data_manager_ios_backend.mm (right):

https://codereview.chromium.org/2945803002/diff/1/ios/web/webui/url_data_mana...
ios/web/webui/url_data_manager_ios_backend.mm:57: bool SchemeIsInSchemes(const
std::string& scheme,
On 2017/06/23 14:31:21, sdefresne wrote:
> ditto, just inline base::ContainsValue in the one callsite

Done.

[sdefresne, 2017-06-27 09:34:36]

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

[sdefresne, 2017-06-27 09:34:37]

Could you please send to ios CQ bots and check that everything compiled cleanly
with the changes and the tests still pass?

https://codereview.chromium.org/2945803002/diff/20001/ios/web/webui/url_data_...
File ios/web/webui/url_data_manager_ios.cc (right):

https://codereview.chromium.org/2945803002/diff/20001/ios/web/webui/url_data_...
ios/web/webui/url_data_manager_ios.cc:133: return
base::ContainsValue(data_sources_, data_source);
data_sources_ is a pointer, base::ContainsValue uses std::begin/std::end but I
don't see any overload that work with a pointer to a collection (according to
http://en.cppreference.com/w/cpp/iterator/begin). I don't see an overload for
base::ContainsValue that takes a pointer to a container.

So I'm surprised this compile and instead I would have expected this to be the
following:

  return base::ContainsValue(*data_sources_, data_source);

Indeed, when I try to apply this change, I get the following compilation error:

../../base/stl_util.h:90:20: error: no matching function for call to 'begin'
  return std::find(std::begin(collection), std::end(collection), value) !=
                   ^~~~~~~~~~

[Tripta, 2017-06-28 07:39:33]

Hi sdefresne,

I have made the changes. I could not check the build because i do not have
permission to run trybots.

https://codereview.chromium.org/2945803002/diff/20001/ios/web/webui/url_data_...
File ios/web/webui/url_data_manager_ios.cc (right):

https://codereview.chromium.org/2945803002/diff/20001/ios/web/webui/url_data_...
ios/web/webui/url_data_manager_ios.cc:133: return
base::ContainsValue(data_sources_, data_source);
On 2017/06/27 09:34:37, sdefresne wrote:
> data_sources_ is a pointer, base::ContainsValue uses std::begin/std::end but I
> don't see any overload that work with a pointer to a collection (according to
> http://en.cppreference.com/w/cpp/iterator/begin). I don't see an overload for
> base::ContainsValue that takes a pointer to a container.
> 
> So I'm surprised this compile and instead I would have expected this to be the
> following:
> 
>   return base::ContainsValue(*data_sources_, data_source);
> 
> Indeed, when I try to apply this change, I get the following compilation
error:
> 
> ../../base/stl_util.h:90:20: error: no matching function for call to 'begin'
>   return std::find(std::begin(collection), std::end(collection), value) !=
>                    ^~~~~~~~~~

Done.

[Tripta, 2017-06-28 07:40:47]

The CQ bit was checked by tripta.g@samsung.com to run a CQ dry run

[Tripta, 2017-06-28 07:40:54]

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

[Tripta, 2017-06-28 07:40:54]

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

[Tripta, 2017-06-28 07:40:54]

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

[commit-bot: I haz the power, 2017-06-28 07:40:56]

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-06-28 07:40:57]

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

[commit-bot: I haz the power, 2017-06-28 07:40:58]

Dry run: No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[sdefresne, 2017-06-28 09:44:03]

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

[commit-bot: I haz the power, 2017-06-28 09:44:12]

Dry run: CQ is trying da patch.

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

[sdefresne, 2017-06-28 09:53:35]

On 2017/06/28 07:39:33, Tripta wrote:
> Hi sdefresne,
> 
> I have made the changes. I could not check the build because i do not have
> permission to run trybots.

I've ran the bot for you. When asking for a review, if you do not have the
privilege to use trybots, you can your reviewer to do it for you.

It looks like there are multiple build failures on iOS. Do you have a setup to
build/test locally your changes?

>
https://codereview.chromium.org/2945803002/diff/20001/ios/web/webui/url_data_...
> File ios/web/webui/url_data_manager_ios.cc (right):
> 
>
https://codereview.chromium.org/2945803002/diff/20001/ios/web/webui/url_data_...
> ios/web/webui/url_data_manager_ios.cc:133: return
> base::ContainsValue(data_sources_, data_source);
> On 2017/06/27 09:34:37, sdefresne wrote:
> > data_sources_ is a pointer, base::ContainsValue uses std::begin/std::end but
I
> > don't see any overload that work with a pointer to a collection (according
to
> > http://en.cppreference.com/w/cpp/iterator/begin). I don't see an overload
for
> > base::ContainsValue that takes a pointer to a container.
> > 
> > So I'm surprised this compile and instead I would have expected this to be
the
> > following:
> > 
> >   return base::ContainsValue(*data_sources_, data_source);
> > 
> > Indeed, when I try to apply this change, I get the following compilation
> error:
> > 
> > ../../base/stl_util.h:90:20: error: no matching function for call to 'begin'
> >   return std::find(std::begin(collection), std::end(collection), value) !=
> >                    ^~~~~~~~~~
> 
> Done.

[commit-bot: I haz the power, 2017-06-28 09:53:41]

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

[commit-bot: I haz the power, 2017-06-28 09:53:42]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[sdefresne, 2017-06-28 12:26:14]

https://codereview.chromium.org/2945803002/diff/60001/ios/chrome/browser/paym...
File ios/chrome/browser/payments/payment_request.mm (right):

https://codereview.chromium.org/2945803002/diff/60001/ios/chrome/browser/paym...
ios/chrome/browser/payments/payment_request.mm:211: if
(base::ContainsValue(supported_cards_networks_, spec_issuer_network)) {
supported_cards_networks_ → supported_card_networks_

[Tripta, 2017-06-28 13:02:01]

Have fixed the error. I have a code setup on linux machine and I use the command
"ninja -C out/Default chrome" to build code. It does not compile files under
ios.

https://codereview.chromium.org/2945803002/diff/60001/ios/chrome/browser/paym...
File ios/chrome/browser/payments/payment_request.mm (right):

https://codereview.chromium.org/2945803002/diff/60001/ios/chrome/browser/paym...
ios/chrome/browser/payments/payment_request.mm:211: if
(base::ContainsValue(supported_cards_networks_, spec_issuer_network)) {
On 2017/06/28 12:26:13, sdefresne wrote:
> supported_cards_networks_ → supported_card_networks_

Done.

[sdefresne, 2017-06-28 16:37:15]

lgtm

Thank you.

Next time, could you ensure the bots are green before asking for review. Maybe
you can find some of you coworker with trybot access and ask them for help.

[Tripta, 2017-06-29 04:50:58]

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

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

CQ is trying da patch.

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

[Tripta, 2017-06-29 04:52:05]

Will do that from next time.

Thanks

[commit-bot: I haz the power, 2017-06-29 05:09:08]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1498711858128280,
"parent_rev": "c09ac82d312a51cee13f376028a3f8d2d6e3fbe6", "commit_rev":
"33e703e1de0b77da838981dae4639e84c72aaea1"}

[commit-bot: I haz the power, 2017-06-29 05:09:21]

Description was changed from

==========
Use ContainsValue() instead of std::find() in ios/

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

to

==========
Use ContainsValue() instead of std::find() in ios/

BUG=561800

Review-Url: https://codereview.chromium.org/2945803002
Cr-Commit-Position: refs/heads/master@{#483300}
Committed:
https://chromium.googlesource.com/chromium/src/+/33e703e1de0b77da838981dae463...
==========

[commit-bot: I haz the power, 2017-06-29 05:09:23]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/33e703e1de0b77da838981dae463...