Add support for loading API keys from Info.plist on iOS and macOS.

Add support for loading API keys from Info.plist on iOS and macOS.

BUG=629511
Committed: https://crrev.com/bd0146024cf0378f3bc4b1a5d9956df319136c5f
Cr-Commit-Position: refs/heads/master@{#411101}

[bzanotti, 2016-08-05 15:41:47]

bzanotti@chromium.org changed reviewers:
+ rogerta@chromium.org

[bzanotti, 2016-08-05 15:41:48]

Please take a first look.

Note that I tested locally, but I'm still trying to figure out how to add a unit
test for this (this requires me to add a test Info.plist or use an existing
one).

[bzanotti, 2016-08-08 16:38:55]

On 2016/08/05 15:41:48, bzanotti wrote:
> Please take a first look.
> 
> Note that I tested locally, but I'm still trying to figure out how to add a
unit
> test for this (this requires me to add a test Info.plist or use an existing
> one).

I figured out a way. :)

This should be ready for review.

[bzanotti, 2016-08-08 16:39:02]

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

[commit-bot: I haz the power, 2016-08-08 16:39:13]

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, 2016-08-08 17:12:41]

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

[commit-bot: I haz the power, 2016-08-08 17:12:42]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Roger Tawa OOO till Jul 10th, 2016-08-08 18:03:05]

The omock code is only for mac/iOS, but I guess there is no way to specify that
in DEPS files?

https://codereview.chromium.org/2224473002/diff/20001/google_apis/google_api_...
File google_apis/google_api_keys_mac_unittest.mm (right):

https://codereview.chromium.org/2224473002/diff/20001/google_apis/google_api_...
google_apis/google_api_keys_mac_unittest.mm:64: }
COMPILE_ASSERT(11 == 3 + 2 * google_apis::CLIENT_NUM_ITEMS];

https://codereview.chromium.org/2224473002/diff/20001/google_apis/google_api_...
File google_apis/google_api_keys_unittest.cc (right):

https://codereview.chromium.org/2224473002/diff/20001/google_apis/google_api_...
google_apis/google_api_keys_unittest.cc:45: #include
"google_apis/google_api_keys_mac.h"
Should this be in an #ifdef?

https://codereview.chromium.org/2224473002/diff/20001/google_apis/google_api_...
google_apis/google_api_keys_unittest.cc:109: };
There seems to be lots of duplicate code here with
google_api_keys_mac_unittest.mm.  Can we refactor this into common code?

[bzanotti, 2016-08-10 13:21:52]

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

[commit-bot: I haz the power, 2016-08-10 13:22:05]

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

[bzanotti, 2016-08-10 13:22:36]

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

[commit-bot: I haz the power, 2016-08-10 13:22:41]

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

[bzanotti, 2016-08-10 13:23:32]

On 2016/08/08 18:03:05, Roger Tawa wrote:
> The omock code is only for mac/iOS, but I guess there is no way to specify
that
> in DEPS files?

I don't know of any way to do that. It shouldn't matter much since all the
headers have Objective-C code in them, and it won't compile in a .cc file.

[bzanotti, 2016-08-10 13:23:40]

PTAL

https://codereview.chromium.org/2224473002/diff/20001/google_apis/google_api_...
File google_apis/google_api_keys_mac_unittest.mm (right):

https://codereview.chromium.org/2224473002/diff/20001/google_apis/google_api_...
google_apis/google_api_keys_mac_unittest.mm:64: }
On 2016/08/08 18:03:05, Roger Tawa wrote:
> COMPILE_ASSERT(11 == 3 + 2 * google_apis::CLIENT_NUM_ITEMS];

Done.

https://codereview.chromium.org/2224473002/diff/20001/google_apis/google_api_...
File google_apis/google_api_keys_unittest.cc (right):

https://codereview.chromium.org/2224473002/diff/20001/google_apis/google_api_...
google_apis/google_api_keys_unittest.cc:45: #include
"google_apis/google_api_keys_mac.h"
On 2016/08/08 18:03:05, Roger Tawa wrote:
> Should this be in an #ifdef?

Yes, this unit test is ran on Linux as well where it makes no sense to include
this header. Done.

https://codereview.chromium.org/2224473002/diff/20001/google_apis/google_api_...
google_apis/google_api_keys_unittest.cc:109: };
On 2016/08/08 18:03:05, Roger Tawa wrote:
> There seems to be lots of duplicate code here with
> google_api_keys_mac_unittest.mm.  Can we refactor this into common code?

Done.

[bzanotti, 2016-08-10 13:52:58]

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

[commit-bot: I haz the power, 2016-08-10 13:53:15]

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

[bzanotti, 2016-08-10 14:28:44]

Patchset #4 (id:60001) has been deleted

[commit-bot: I haz the power, 2016-08-10 14:29:53]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[bzanotti, 2016-08-10 14:30:22]

The CQ bit was unchecked by bzanotti@chromium.org

[bzanotti, 2016-08-10 14:31:27]

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

[commit-bot: I haz the power, 2016-08-10 14:32:07]

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

[Roger Tawa OOO till Jul 10th, 2016-08-10 14:40:12]

lgtm

Thanks Benoit.

[bzanotti, 2016-08-10 14:54:06]

bzanotti@chromium.org changed reviewers:
+ rsesek@chromium.org

[bzanotti, 2016-08-10 14:54:07]

Robert, please do an OWNERS review of the DEPS change.

[Robert Sesek, 2016-08-10 15:04:17]

LGTM

https://codereview.chromium.org/2224473002/diff/80001/google_apis/google_api_...
File google_apis/google_api_keys_mac_unittest.mm (right):

https://codereview.chromium.org/2224473002/diff/80001/google_apis/google_api_...
google_apis/google_api_keys_mac_unittest.mm:1: // Copyright (c) 2016 The
Chromium Authors. All rights reserved.
no (c) in new files

https://codereview.chromium.org/2224473002/diff/80001/google_apis/google_api_...
File google_apis/google_api_keys_unittest.h (right):

https://codereview.chromium.org/2224473002/diff/80001/google_apis/google_api_...
google_apis/google_api_keys_unittest.h:16: public:
structs are public by default

https://codereview.chromium.org/2224473002/diff/80001/google_apis/google_api_...
google_apis/google_api_keys_unittest.h:17: EnvironmentCache() :
variable_name(NULL), was_set(false) {}
NULL -> nullptr

[commit-bot: I haz the power, 2016-08-10 15:50:16]

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

[commit-bot: I haz the power, 2016-08-10 15:50:17]

Dry run: This issue passed the CQ dry run.

[bzanotti, 2016-08-10 16:25:00]

Thanks for the review.

https://codereview.chromium.org/2224473002/diff/80001/google_apis/google_api_...
File google_apis/google_api_keys_mac_unittest.mm (right):

https://codereview.chromium.org/2224473002/diff/80001/google_apis/google_api_...
google_apis/google_api_keys_mac_unittest.mm:1: // Copyright (c) 2016 The
Chromium Authors. All rights reserved.
On 2016/08/10 15:04:16, Robert Sesek wrote:
> no (c) in new files

Done.

https://codereview.chromium.org/2224473002/diff/80001/google_apis/google_api_...
File google_apis/google_api_keys_unittest.h (right):

https://codereview.chromium.org/2224473002/diff/80001/google_apis/google_api_...
google_apis/google_api_keys_unittest.h:16: public:
On 2016/08/10 15:04:16, Robert Sesek wrote:
> structs are public by default

Done.

https://codereview.chromium.org/2224473002/diff/80001/google_apis/google_api_...
google_apis/google_api_keys_unittest.h:17: EnvironmentCache() :
variable_name(NULL), was_set(false) {}
On 2016/08/10 15:04:16, Robert Sesek wrote:
> NULL -> nullptr

Done.

[bzanotti, 2016-08-10 16:25:04]

The CQ bit was checked by bzanotti@chromium.org

[bzanotti, 2016-08-10 16:25:05]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
rogerta@chromium.org
Link to the patchset: https://codereview.chromium.org/2224473002/#ps100001
(title: "Fix nits")

[commit-bot: I haz the power, 2016-08-10 16:25:35]

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, 2016-08-10 18:43:26]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-08-10 18:47:21]

Description was changed from

==========
Add support for loading API keys from Info.plist on iOS and macOS.

BUG=629511
==========

to

==========
Add support for loading API keys from Info.plist on iOS and macOS.

BUG=629511

Committed: https://crrev.com/bd0146024cf0378f3bc4b1a5d9956df319136c5f
Cr-Commit-Position: refs/heads/master@{#411101}
==========

[commit-bot: I haz the power, 2016-08-10 18:47:22]

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