[ios] Adds BrandedImageProvider and its associated image resources.

[ios] Adds BrandedImageProvider and its associated image resources.

This provider will vend images that contain embedder-specific branding.

BUG=665967
Committed: https://crrev.com/2c8da2b13c765261c6c4890c8386d2cdd4d74201
Cr-Commit-Position: refs/heads/master@{#433862}

[rohitrao (ping after 24h), 2016-11-21 21:02:10]

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

[commit-bot: I haz the power, 2016-11-21 21:03:12]

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

[rohitrao (ping after 24h), 2016-11-21 21:06:23]

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

[rohitrao (ping after 24h), 2016-11-21 21:06:24]

I'm still adding more methods to the provider, but this is ready for an initial
look.  The remaining methods will all look very similar to the existing UIImage
methods.

[commit-bot: I haz the power, 2016-11-21 23:33:50]

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

[commit-bot: I haz the power, 2016-11-21 23:33:51]

Dry run: This issue passed the CQ dry run.

[rohitrao (ping after 24h), 2016-11-22 12:17:22]

This is ready for review now.  I believe this is the full set of methods that we
will need.

[sdefresne, 2016-11-22 13:29:31]

lgtm with comments

https://codereview.chromium.org/2521493003/diff/60001/ios/chrome/browser/prov...
File ios/chrome/browser/providers/BUILD.gn (right):

https://codereview.chromium.org/2521493003/diff/60001/ios/chrome/browser/prov...
ios/chrome/browser/providers/BUILD.gn:15:
"images/chromium_branded_image_provider.h",
Can you move this to //ios/chrome/browser/providers/images/BUILD.gn?

https://codereview.chromium.org/2521493003/diff/60001/ios/public/provider/chr...
File ios/public/provider/chrome/browser/images/test_branded_image_provider.h
(right):

https://codereview.chromium.org/2521493003/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/images/test_branded_image_provider.h:16:
UIImage* GetAccountsListActivityControlsImage() override;
Do you really need to override those? The implementation appears to be the same
(return nil/false) as the super class.

[rohitrao (ping after 24h), 2016-11-22 13:33:47]

I have a general question about whether we want our test providers to be dense
or sparse.

https://codereview.chromium.org/2521493003/diff/60001/ios/public/provider/chr...
File ios/public/provider/chrome/browser/images/test_branded_image_provider.h
(right):

https://codereview.chromium.org/2521493003/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/images/test_branded_image_provider.h:16:
UIImage* GetAccountsListActivityControlsImage() override;
On 2016/11/22 13:29:30, sdefresne wrote:
> Do you really need to override those? The implementation appears to be the
same
> (return nil/false) as the super class.

This test provider has no subclass-specific behavior at all.  Every method
returns nil or false.

Should I just remove this class entirely and use an instance of the superclass
instead?  Is there value in having this shell, or should we only add it if/when
we need it in the future?

I've added a number of other mostly-empty test providers.  Should I delete the
ones that do nothing?  Should I remove method overrides if they do the same
thing as the superclass implementation?

[sdefresne, 2016-11-22 13:41:27]

https://codereview.chromium.org/2521493003/diff/60001/ios/public/provider/chr...
File ios/public/provider/chrome/browser/images/test_branded_image_provider.h
(right):

https://codereview.chromium.org/2521493003/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/images/test_branded_image_provider.h:16:
UIImage* GetAccountsListActivityControlsImage() override;
On 2016/11/22 13:33:47, rohitrao wrote:
> On 2016/11/22 13:29:30, sdefresne wrote:
> > Do you really need to override those? The implementation appears to be the
> same
> > (return nil/false) as the super class.
> 
> This test provider has no subclass-specific behavior at all.  Every method
> returns nil or false.
> 
> Should I just remove this class entirely and use an instance of the superclass
> instead?  Is there value in having this shell, or should we only add it
if/when
> we need it in the future?

I'd prefer to keep this class as having a sub-class TestBrandedImageProvider
gives the tiny benefit that when/if we want to have a different implementation
for the tests than the default, we do not have to hunt for all the usages of the
base class in the tests.

> I've added a number of other mostly-empty test providers.  Should I delete the
> ones that do nothing?  Should I remove method overrides if they do the same
> thing as the superclass implementation?

I think we should remove overrides that are identical to super-class but keep
the classes for the same reason.

[rohitrao (ping after 24h), 2016-11-22 13:43:02]

https://codereview.chromium.org/2521493003/diff/60001/ios/public/provider/chr...
File ios/public/provider/chrome/browser/images/test_branded_image_provider.h
(right):

https://codereview.chromium.org/2521493003/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/images/test_branded_image_provider.h:16:
UIImage* GetAccountsListActivityControlsImage() override;
> I think we should remove overrides that are identical to super-class but keep
> the classes for the same reason.

SG.  I'll update this CL and fix up the other test providers in a followup.

[rohitrao (ping after 24h), 2016-11-22 13:56:08]

https://codereview.chromium.org/2521493003/diff/60001/ios/chrome/browser/prov...
File ios/chrome/browser/providers/BUILD.gn (right):

https://codereview.chromium.org/2521493003/diff/60001/ios/chrome/browser/prov...
ios/chrome/browser/providers/BUILD.gn:15:
"images/chromium_branded_image_provider.h",
On 2016/11/22 13:29:30, sdefresne wrote:
> Can you move this to //ios/chrome/browser/providers/images/BUILD.gn?

Done.

[rohitrao (ping after 24h), 2016-11-22 13:56:13]

The CQ bit was checked by rohitrao@chromium.org

[rohitrao (ping after 24h), 2016-11-22 13:56:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2521493003/#ps80001
(title: "Review")

[commit-bot: I haz the power, 2016-11-22 13:56:36]

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-11-22 15:19:56]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1479822973830000,
"parent_rev": "1bae4b647133347795a2e49bf9ec36ad05c64a15", "commit_rev":
"60fa8dd73bae3b37a49a6f02b3458d90595f1a60"}

[commit-bot: I haz the power, 2016-11-22 15:20:21]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-22 15:22:52]

Description was changed from

==========
[ios] Adds BrandedImageProvider and its associated image resources.

This provider will vend images that contain embedder-specific branding.

BUG=665967
==========

to

==========
[ios] Adds BrandedImageProvider and its associated image resources.

This provider will vend images that contain embedder-specific branding.

BUG=665967

Committed: https://crrev.com/2c8da2b13c765261c6c4890c8386d2cdd4d74201
Cr-Commit-Position: refs/heads/master@{#433862}
==========

[commit-bot: I haz the power, 2016-11-22 15:22:54]

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