Android: Mark some profile code as non-mobile.

Android: Mark some profile code as non-mobile.

Go up the call chain and mark some more code as non-mobile.

BUG=159847
Committed: https://crrev.com/af58280da7183e3806d05b50fb2b24d3ee8dcd96
Cr-Commit-Position: refs/heads/master@{#352094}

[Lei Zhang, 2015-10-01 07:51:41]

thestig@chromium.org changed reviewers:
+ mlerman@chromium.org

[Lei Zhang, 2015-10-01 08:19:05]

Patchset #1 (id:1) has been deleted

[Mike Lerman, 2015-10-01 13:50:57]

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

[Mike Lerman, 2015-10-01 13:50:57]

as far as I understand, while iOS is phasing out multi-profile support, that
hasn't happened yet. Benoit, can you comment? Can we take these multi-profile
functions away from IOS?

[bzanotti, 2015-10-01 13:58:31]

On 2015/10/01 13:50:57, Mike Lerman wrote:
> as far as I understand, while iOS is phasing out multi-profile support, that
> hasn't happened yet. Benoit, can you comment? Can we take these multi-profile
> functions away from IOS?

You understood correctly, multi-profile support is being phased out but it
hasn't happened yet.

That being said, Chrome iOS isn't supposed to use code in chrome/, adding guards
for iOS in there is thus not necessary. This CL could safely just exclude
Android exclusively.

[Lei Zhang, 2015-10-01 18:36:28]

On 2015/10/01 13:58:31, bzanotti wrote:
> On 2015/10/01 13:50:57, Mike Lerman wrote:
> > as far as I understand, while iOS is phasing out multi-profile support, that
> > hasn't happened yet. Benoit, can you comment? Can we take these
multi-profile
> > functions away from IOS?
> 
> You understood correctly, multi-profile support is being phased out but it
> hasn't happened yet.
> 
> That being said, Chrome iOS isn't supposed to use code in chrome/, adding
guards
> for iOS in there is thus not necessary. This CL could safely just exclude
> Android exclusively.

I checked my downstream iOS build - profile_manager.cc and profile_metrics.cc
are still being compiled on iOS, whereas profile_window.cc is not. Thus the
OS_IOS checks. When iOS does get out of chrome/, someone can just sweep through
and remove all the OS_IOS checks.

[bzanotti, 2015-10-02 11:56:41]

On 2015/10/01 18:36:28, Lei Zhang wrote:
> On 2015/10/01 13:58:31, bzanotti wrote:
> > On 2015/10/01 13:50:57, Mike Lerman wrote:
> > > as far as I understand, while iOS is phasing out multi-profile support,
that
> > > hasn't happened yet. Benoit, can you comment? Can we take these
> multi-profile
> > > functions away from IOS?
> > 
> > You understood correctly, multi-profile support is being phased out but it
> > hasn't happened yet.
> > 
> > That being said, Chrome iOS isn't supposed to use code in chrome/, adding
> guards
> > for iOS in there is thus not necessary. This CL could safely just exclude
> > Android exclusively.
> 
> I checked my downstream iOS build - profile_manager.cc and profile_metrics.cc
> are still being compiled on iOS, whereas profile_window.cc is not. Thus the
> OS_IOS checks. When iOS does get out of chrome/, someone can just sweep
through
> and remove all the OS_IOS checks.

Yes, some of it is still used, we just tend to try to keep this from spilling in
chrome/ and just handle that downstream.
I cc'd David (droger@) to weight in on this, as he (and his team) is working on
getting iOS out of chrome/

[droger, 2015-10-02 12:09:38]

The usage of //chrome by iOS is deprecating, so we don't generally encourage
adding iOS specific code there to avoid confusing developers.

That said, it can be fine to do it in specific places, if the resulting code is
better/clearer. I don't have a strong opinion, I'd say it's up to the owner.

[Lei Zhang, 2015-10-02 17:03:45]

On 2015/10/02 12:09:38, droger wrote:
> The usage of //chrome by iOS is deprecating, so we don't generally encourage
> adding iOS specific code there to avoid confusing developers.
> 
> That said, it can be fine to do it in specific places, if the resulting code
is
> better/clearer. I don't have a strong opinion, I'd say it's up to the owner.

Well, I'm adding a !defined(OS_IOS) here, so I'm actually removing bits of
potential iOS chrome/ usage here. If I see a file is not being built on iOS,
then I don't bother adding !defined(OS_IOS).

[Mike Lerman, 2015-10-02 18:06:31]

On 2015/10/02 17:03:45, Lei Zhang wrote:
> On 2015/10/02 12:09:38, droger wrote:
> > The usage of //chrome by iOS is deprecating, so we don't generally encourage
> > adding iOS specific code there to avoid confusing developers.
> > 
> > That said, it can be fine to do it in specific places, if the resulting code
> is
> > better/clearer. I don't have a strong opinion, I'd say it's up to the owner.
> 
> Well, I'm adding a !defined(OS_IOS) here, so I'm actually removing bits of
> potential iOS chrome/ usage here. If I see a file is not being built on iOS,
> then I don't bother adding !defined(OS_IOS).

I think in this case, the !defined(OS_IOS) code would be more helpful than
harmful. It can be hard to track what aspects of profiles/ code is used on which
platforms. Even though for iOS developers they know that /chrome isn't the place
for finding code for their platform, that's not always obvious to non-IOS
developers.

LGTM as is.

[Mike Lerman, 2015-10-02 18:06:43]

And thanks everyone for weighing in!

[Lei Zhang, 2015-10-02 18:16:37]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2015-10-02 18:17:16]

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

[commit-bot: I haz the power, 2015-10-02 18:56:59]

Committed patchset #1 (id:20001)

[commit-bot: I haz the power, 2015-10-02 18:57:50]

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