Exclude building signin view controller and its delegate on ChromeOS

Exclude building signin view controller and its delegate on ChromeOS

The sign-in dialogs managed by the sign-in view controller are never
used on ChromeOS. This CL avoids building them on ChromeOS.

This is needed in order to be able to re-land the change in CL
https://codereview.chromium.org/2771113003 as that change requires
a dependency between signin_view_controller_delegate_views.cc and
sync_confirmation_ui.cc.

BUG=694476
Review-Url: https://codereview.chromium.org/2785753003
Cr-Commit-Position: refs/heads/master@{#460829}
Committed: https://chromium.googlesource.com/chromium/src/+/c6841193b24c0dfd6e51ed82eaab5b4c33ae3ea9

[msarda, 2017-03-29 16:01:31]

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

[msarda, 2017-03-29 16:02:09]

msarda@chromium.org changed reviewers:
+ anthonyvd@chromium.org

[commit-bot: I haz the power, 2017-03-29 16:02:10]

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

[msarda, 2017-03-29 16:02:30]

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

[commit-bot: I haz the power, 2017-03-29 16:02:36]

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

[msarda, 2017-03-29 16:02:49]

msarda@chromium.org changed reviewers:
+ pkasting@chromium.org

[msarda, 2017-03-29 16:02:49]

pkasting@chromium.org: Please do an OWNERs review of this CL. Thank you.

[commit-bot: I haz the power, 2017-03-29 17:03:46]

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

[commit-bot: I haz the power, 2017-03-29 17:03:46]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-03-29 19:39:32]

https://codereview.chromium.org/2785753003/diff/1/chrome/browser/ui/signin_vi...
File chrome/browser/ui/signin_view_controller.h (right):

https://codereview.chromium.org/2785753003/diff/1/chrome/browser/ui/signin_vi...
chrome/browser/ui/signin_view_controller.h:22: // mobile or ChromeOS)
Nit: Since CrOS is a desktop platform, this is a bit confusing; just say "Note:
This class is not used on mobile or CrOS."

I'd say "not compiled in", but this is compiled in, in stub form, on CrOS after
this patch.  Is it feasible to make it not be compiled-in at all?  Can we have
like a #if USE_SIGNIN that guards access to this type, so it's obvious to
readers of calling code that said code is not always functional?  Maybe those
callers are already not compiled in on the target platforms?

[msarda, 2017-03-30 11:52:40]

Patchset #2 (id:20001) has been deleted

[msarda, 2017-03-30 11:52:45]

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

[commit-bot: I haz the power, 2017-03-30 11:53:00]

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

[msarda, 2017-03-30 11:54:11]

I managed to exclude entirely the build of SigninViewController (.h and .cc
files) on ChromeOS.

Please take another look.

https://codereview.chromium.org/2785753003/diff/1/chrome/browser/ui/signin_vi...
File chrome/browser/ui/signin_view_controller.h (right):

https://codereview.chromium.org/2785753003/diff/1/chrome/browser/ui/signin_vi...
chrome/browser/ui/signin_view_controller.h:22: // mobile or ChromeOS)
On 2017/03/29 19:39:31, Peter Kasting wrote:
> Nit: Since CrOS is a desktop platform, this is a bit confusing; just say
"Note:
> This class is not used on mobile or CrOS."
> 
> I'd say "not compiled in", but this is compiled in, in stub form, on CrOS
after
> this patch.  Is it feasible to make it not be compiled-in at all?  Can we have
> like a #if USE_SIGNIN that guards access to this type, so it's obvious to
> readers of calling code that said code is not always functional?  Maybe those
> callers are already not compiled in on the target platforms?

I excluded this file on ChromeOS entirely and removed this comment.

[commit-bot: I haz the power, 2017-03-30 12:08:34]

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

[commit-bot: I haz the power, 2017-03-30 12:08:34]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[msarda, 2017-03-30 12:25:39]

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

[commit-bot: I haz the power, 2017-03-30 12:25:58]

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

[msarda, 2017-03-30 12:42:50]

Patchset #2 (id:40001) has been deleted

[msarda, 2017-03-30 12:42:59]

Patchset #2 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-03-30 13:28:39]

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

[commit-bot: I haz the power, 2017-03-30 13:28:40]

Dry run: This issue passed the CQ dry run.

[anthonyvd, 2017-03-30 14:36:25]

lgtm

[msarda, 2017-03-30 14:51:58]

Description was changed from

==========
Exclude building signin view controller and its delegate on ChromeOS

The sign-in dialogs managed by the sign-in view controller are never
used on ChromeOS. This CL avoids building them on ChromeOS.

This is needed in order to be able to land the change in CL
https://codereview.chromium.org/2784823002/ as that change requires
a dependency between signin_view_controller_delegate_views.cc and
sync_confirmation_ui.cc.

BUG=694476
==========

to

==========
Exclude building signin view controller and its delegate on ChromeOS

The sign-in dialogs managed by the sign-in view controller are never
used on ChromeOS. This CL avoids building them on ChromeOS.

This is needed in order to be able to re-land the change in CL
https://codereview.chromium.org/2771113003 as that change requires
a dependency between signin_view_controller_delegate_views.cc and
sync_confirmation_ui.cc.

BUG=694476
==========

[Peter Kasting, 2017-03-30 17:33:13]

LGTM

[msarda, 2017-03-30 18:15:00]

The CQ bit was checked by msarda@chromium.org

[commit-bot: I haz the power, 2017-03-30 18:15:29]

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, 2017-03-30 18:24:46]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490897700098170,
"parent_rev": "bc85b8b89d33b7a7b8da86268fc30ba4fac70c87", "commit_rev":
"c6841193b24c0dfd6e51ed82eaab5b4c33ae3ea9"}

[commit-bot: I haz the power, 2017-03-30 18:25:37]

Description was changed from

==========
Exclude building signin view controller and its delegate on ChromeOS

The sign-in dialogs managed by the sign-in view controller are never
used on ChromeOS. This CL avoids building them on ChromeOS.

This is needed in order to be able to re-land the change in CL
https://codereview.chromium.org/2771113003 as that change requires
a dependency between signin_view_controller_delegate_views.cc and
sync_confirmation_ui.cc.

BUG=694476
==========

to

==========
Exclude building signin view controller and its delegate on ChromeOS

The sign-in dialogs managed by the sign-in view controller are never
used on ChromeOS. This CL avoids building them on ChromeOS.

This is needed in order to be able to re-land the change in CL
https://codereview.chromium.org/2771113003 as that change requires
a dependency between signin_view_controller_delegate_views.cc and
sync_confirmation_ui.cc.

BUG=694476

Review-Url: https://codereview.chromium.org/2785753003
Cr-Commit-Position: refs/heads/master@{#460829}
Committed:
https://chromium.googlesource.com/chromium/src/+/c6841193b24c0dfd6e51ed82eaab...
==========

[commit-bot: I haz the power, 2017-03-30 18:25:38]

Committed patchset #2 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/c6841193b24c0dfd6e51ed82eaab...