Create stub jingle library for iOS.

Create stub jingle library for iOS.

 iOS doesn't use libjingle, but needs a stub jingle library for sync to
compile.

BUG=143886
R=akalin@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=152975

[akalin, 2012-08-20 18:51:14]

also, can you create a bug for this issue?  something like 'separate out
os-specific parts of notifier' or something

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp
File jingle/jingle.gyp (right):

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode215
jingle/jingle.gyp:215: {
do you need to stub out jingle_glue?  All its dependents are in this file, and
those are already not compiled in OS X, right?

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode220
jingle/jingle.gyp:220: 'target_name': 'notifier',
can you do the os conditions inside the notifier target?  or at least put them
right next to each other.

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode234
jingle/jingle.gyp:234: 'target_name': 'notifier_test_util',
i'm curious how this works.  sync_notifier_tests depend on this, so how does it
still compile?  is it not compiled on ios?  if so, i prefer not having this stub
here at all.

http://codereview.chromium.org/10829424/diff/1/jingle/notifier/base/notifier_...
File jingle/notifier/base/notifier_options_ios.cc (right):

http://codereview.chromium.org/10829424/diff/1/jingle/notifier/base/notifier_...
jingle/notifier/base/notifier_options_ios.cc:12:
NotifierOptions::~NotifierOptions() {
i think you can avoid creating this stub file.  The only thing
notifier_options.cc depends on from gaia_token_pre_xmpp_auth.h is the
kDefaultAuthMechanism constant.

Perhaps make a jingle/notifier/base/gaia_constants.h file and put
kDefaultAuthMechanism in there.  Then you can just use notifier_options.cc.

[qsr, 2012-08-21 08:21:17]

+ajwong@chromium.org for remoting/

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp
File jingle/jingle.gyp (right):

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode215
jingle/jingle.gyp:215: {
 jingle_glue is referenced in sync.gyp, so I need it to exist for ios too.

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode220
jingle/jingle.gyp:220: 'target_name': 'notifier',
I could, but I need the other targets too, as they are referenced in sync.gyp.
Do you want me to put 4 conditionals instead of 1, 1 per target? Because most of
the target content will be in a conditional then.

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode234
jingle/jingle.gyp:234: 'target_name': 'notifier_test_util',
On 2012/08/20 18:51:14, akalin wrote:
> i'm curious how this works.  sync_notifier_tests depend on this, so how does
it
> still compile?  is it not compiled on ios?  if so, i prefer not having this
stub
> here at all.

 I don't understand the question. sync_notifier_tests depend on this, so it must
exist. Now, every test that depends on jingle is not compiled for iOS as we do
not have libjingle at all.

http://codereview.chromium.org/10829424/diff/1/jingle/notifier/base/notifier_...
File jingle/notifier/base/notifier_options_ios.cc (right):

http://codereview.chromium.org/10829424/diff/1/jingle/notifier/base/notifier_...
jingle/notifier/base/notifier_options_ios.cc:12:
NotifierOptions::~NotifierOptions() {
 Excellent idea. Thanks.

[akalin, 2012-08-22 19:46:26]

lgtm

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp
File jingle/jingle.gyp (right):

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode215
jingle/jingle.gyp:215: {
On 2012/08/21 08:21:17, qsr wrote:
>  jingle_glue is referenced in sync.gyp, so I need it to exist for ios too.

ah, you're right, my mistake.

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode220
jingle/jingle.gyp:220: 'target_name': 'notifier',
On 2012/08/21 08:21:17, qsr wrote:
> I could, but I need the other targets too, as they are referenced in sync.gyp.
> Do you want me to put 4 conditionals instead of 1, 1 per target? Because most
of
> the target content will be in a conditional then.

Per your reply to my other comments, I think this is okay as is.

http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode234
jingle/jingle.gyp:234: 'target_name': 'notifier_test_util',
On 2012/08/21 08:21:17, qsr wrote:
> On 2012/08/20 18:51:14, akalin wrote:
> > i'm curious how this works.  sync_notifier_tests depend on this, so how does
> it
> > still compile?  is it not compiled on ios?  if so, i prefer not having this
> stub
> > here at all.
> 
>  I don't understand the question. sync_notifier_tests depend on this, so it
must
> exist. Now, every test that depends on jingle is not compiled for iOS as we do
> not have libjingle at all.

I guess gyp would complain if the target isn't there, even if its dependents
aren't compiled.  Fair enough.

http://codereview.chromium.org/10829424/diff/2003/jingle/notifier/base/gaia_c...
File jingle/notifier/base/gaia_constants.h (right):

http://codereview.chromium.org/10829424/diff/2003/jingle/notifier/base/gaia_c...
jingle/notifier/base/gaia_constants.h:10: namespace gaia_constants {
the extra namespace is probably overkill.  perhaps instead rename
kDefaultAuthMechanism to kDefaultGaiaAuthMechanism?

[awong, 2012-08-22 19:53:30]

FYI, wez@ is better for remoting chagnes as I am no longer on that team.

On Tue, Aug 21, 2012 at 1:21 AM, <qsr@chromium.org> wrote:

> +ajwong@chromium.org for remoting/
>
>
>
http://codereview.chromium.**org/10829424/diff/1/jingle/**jingle.gyp<http://c...
> File jingle/jingle.gyp (right):
>
> http://codereview.chromium.**org/10829424/diff/1/jingle/**
>
jingle.gyp#newcode215<http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode215>
> jingle/jingle.gyp:215: {
>  jingle_glue is referenced in sync.gyp, so I need it to exist for ios
> too.
>
> http://codereview.chromium.**org/10829424/diff/1/jingle/**
>
jingle.gyp#newcode220<http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode220>
> jingle/jingle.gyp:220: 'target_name': 'notifier',
> I could, but I need the other targets too, as they are referenced in
> sync.gyp. Do you want me to put 4 conditionals instead of 1, 1 per
> target? Because most of the target content will be in a conditional
> then.
>
> http://codereview.chromium.**org/10829424/diff/1/jingle/**
>
jingle.gyp#newcode234<http://codereview.chromium.org/10829424/diff/1/jingle/jingle.gyp#newcode234>
> jingle/jingle.gyp:234: 'target_name': 'notifier_test_util',
> On 2012/08/20 18:51:14, akalin wrote:
>
>> i'm curious how this works.  sync_notifier_tests depend on this, so
>>
> how does it
>
>> still compile?  is it not compiled on ios?  if so, i prefer not having
>>
> this stub
>
>> here at all.
>>
>
>  I don't understand the question. sync_notifier_tests depend on this, so
> it must exist. Now, every test that depends on jingle is not compiled
> for iOS as we do not have libjingle at all.
>
> http://codereview.chromium.**org/10829424/diff/1/jingle/**
>
notifier/base/notifier_**options_ios.cc<http://codereview.chromium.org/10829424/diff/1/jingle/notifier/base/notifier_options_ios.cc>
> File jingle/notifier/base/notifier_**options_ios.cc (right):
>
> http://codereview.chromium.**org/10829424/diff/1/jingle/**
>
notifier/base/notifier_**options_ios.cc#newcode12<http://codereview.chromium.org/10829424/diff/1/jingle/notifier/base/notifier_options_ios.cc#newcode12>
> jingle/notifier/base/notifier_**options_ios.cc:12:
> NotifierOptions::~**NotifierOptions() {
>  Excellent idea. Thanks.
>
>
http://codereview.chromium.**org/10829424/<http://codereview.chromium.org/108...
>

[Wez, 2012-08-22 21:20:01]

-awong@

remoting/ LGTM

[qsr, 2012-08-23 07:37:06]

https://chromiumcodereview.appspot.com/10829424/diff/2003/jingle/notifier/bas...
File jingle/notifier/base/gaia_constants.h (right):

https://chromiumcodereview.appspot.com/10829424/diff/2003/jingle/notifier/bas...
jingle/notifier/base/gaia_constants.h:10: namespace gaia_constants {
On 2012/08/22 19:46:26, akalin wrote:
> the extra namespace is probably overkill.  perhaps instead rename
> kDefaultAuthMechanism to kDefaultGaiaAuthMechanism?

Done.

[commit-bot: I haz the power, 2012-08-23 07:37:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qsr@chromium.org/10829424/16001

[commit-bot: I haz the power, 2012-08-23 09:59:04]

Change committed as 152975