handoff: Fix the origin so that it correctly reflects the sender.

handoff: Fix the origin so that it correctly reflects the sender.

This broke when I migrated the Handoff Manager from chrome/ into
components/

BUG=

Committed: https://crrev.com/56915573d8e4be522d4bd8aea1bf8ba76a3c8f77
Cr-Commit-Position: refs/heads/master@{#322581}

[erikchen, 2015-03-26 17:03:59]

erikchen@chromium.org changed reviewers:
+ avi@chromium.org

[erikchen, 2015-03-26 17:04:00]

avi: Please review.

[Avi (use Gerrit), 2015-03-26 18:14:05]

lgtm

https://codereview.chromium.org/1000723003/diff/1/components/handoff/handoff_...
File components/handoff/handoff_manager.mm (right):

https://codereview.chromium.org/1000723003/diff/1/components/handoff/handoff_...
components/handoff/handoff_manager.mm:44: self = [super init];
Can we #ifdef so that if this is built for iOS we automatically get ORIGIN_IOS
and if built for Mac we get ORIGIN_MAC?

[erikchen, 2015-03-26 18:22:36]

https://codereview.chromium.org/1000723003/diff/1/components/handoff/handoff_...
File components/handoff/handoff_manager.mm (right):

https://codereview.chromium.org/1000723003/diff/1/components/handoff/handoff_...
components/handoff/handoff_manager.mm:44: self = [super init];
On 2015/03/26 18:14:04, Avi wrote:
> Can we #ifdef so that if this is built for iOS we automatically get ORIGIN_IOS
> and if built for Mac we get ORIGIN_MAC?

Good suggestion, done.

[Avi (use Gerrit), 2015-03-26 18:40:21]

still lgtm

https://codereview.chromium.org/1000723003/diff/20001/components/handoff/hand...
File components/handoff/handoff_manager.mm (right):

https://codereview.chromium.org/1000723003/diff/20001/components/handoff/hand...
components/handoff/handoff_manager.mm:44: _origin = handoff::ORIGIN_IOS;
#if defined(OS_IOS)
    _origin = handoff::ORIGIN_IOS;
#elif defined(OS_MACOSX)
    _origin = handoff::ORIGIN_MAC;

?

[erikchen, 2015-03-27 14:52:24]

https://codereview.chromium.org/1000723003/diff/20001/components/handoff/hand...
File components/handoff/handoff_manager.mm (right):

https://codereview.chromium.org/1000723003/diff/20001/components/handoff/hand...
components/handoff/handoff_manager.mm:44: _origin = handoff::ORIGIN_IOS;
On 2015/03/26 18:40:21, Avi wrote:
> #if defined(OS_IOS)
>     _origin = handoff::ORIGIN_IOS;
> #elif defined(OS_MACOSX)
>     _origin = handoff::ORIGIN_MAC;
> 
> ?

I'm generally a fan of shorter code, but in this case I prefer being more
verbose. It's unclear that OS_MACOSX can be defined while OS_IOS is also
defined. In your suggestion, it would be very easy for someone to reverse the
#if/#elif, and not realize that they violated an implicit dependency between the
conditionals. We could add comments, but that would yield more code than what I
have right now.

[erikchen, 2015-03-27 14:52:30]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2015-03-27 14:52:47]

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

[Avi (use Gerrit), 2015-03-27 15:16:39]

https://codereview.chromium.org/1000723003/diff/20001/components/handoff/hand...
File components/handoff/handoff_manager.mm (right):

https://codereview.chromium.org/1000723003/diff/20001/components/handoff/hand...
components/handoff/handoff_manager.mm:44: _origin = handoff::ORIGIN_IOS;
On 2015/03/27 14:52:24, erikchen wrote:
> On 2015/03/26 18:40:21, Avi wrote:
> > #if defined(OS_IOS)
> >     _origin = handoff::ORIGIN_IOS;
> > #elif defined(OS_MACOSX)
> >     _origin = handoff::ORIGIN_MAC;
> > 
> > ?
> 
> I'm generally a fan of shorter code, but in this case I prefer being more
> verbose. It's unclear that OS_MACOSX can be defined while OS_IOS is also
> defined. In your suggestion, it would be very easy for someone to reverse the
> #if/#elif, and not realize that they violated an implicit dependency between
the
> conditionals. We could add comments, but that would yield more code than what
I
> have right now.

That makes sense.

[commit-bot: I haz the power, 2015-03-27 15:29:48]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-27 15:30:38]

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