Fixing signed in message and disconnect/reconnect case.

remoting/ios/app/remoting_view_controller.mm

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
 #import "remoting/ios/app/remoting_view_controller.h"
 
@@ skipping 14 matching lines @@
 
 #include "base/strings/sys_string_conversions.h"
 #include "remoting/base/oauth_token_getter.h"
 #include "remoting/client/connect_to_host_info.h"
 
 static CGFloat kHostInset = 5.f;
 
 @interface RemotingViewController ()<HostCollectionViewControllerDelegate,
                                      UIViewControllerAnimatedTransitioning,
                                      UIViewControllerTransitioningDelegate> {
-  bool _isAuthenticated;
+  BOOL _isAuthenticated;
+  BOOL _showAccountSignIn;

[Yuwei, 2017/07/06 22:29:45]

Maybe change this to _showSignedInAccount? _showAccountSignIn sounds like it
will decide whether the account sign in flow should present...

[nicholss, 2017/07/06 22:40:01]

sounds fine. Done.

   MDCDialogTransitionController* _dialogTransitionController;
   MDCAppBar* _appBar;
   HostCollectionViewController* _collectionViewController;
   RemotingService* _remotingService;
 }
 @end
 
 // TODO(nicholss): Localize this file.
 // TODO(nicholss): This file is not finished with integration, the app flow is
 // still pending development.
 
 @implementation RemotingViewController
 
 - (instancetype)init {
   _isAuthenticated = NO;
+  _showAccountSignIn = YES;
   UICollectionViewFlowLayout* layout =
       [[MDCCollectionViewFlowLayout alloc] init];
   layout.minimumInteritemSpacing = 0;
   CGFloat sectionInset = kHostInset * 2.f;
   [layout setSectionInset:UIEdgeInsetsMake(sectionInset, sectionInset,
                                            sectionInset, sectionInset)];
   HostCollectionViewController* collectionVC = [
       [HostCollectionViewController alloc] initWithCollectionViewLayout:layout];
   self = [super initWithContentViewController:collectionVC];
   if (self) {
@@ skipping 58 matching lines @@
          selector:@selector(hostsDidUpdateNotification:)
              name:kHostsDidUpdate
            object:nil];
   [[NSNotificationCenter defaultCenter]
       addObserver:self
          selector:@selector(userDidUpdateNotification:)
              name:kUserDidUpdate
            object:nil];
 }
 
-- (void)viewWillAppear:(BOOL)animated {
-  [super viewWillAppear:animated];
-
-  [self nowAuthenticated:_remotingService.authentication.user.isAuthenticated];
-  [self presentStatus];
-}
-
 - (void)viewDidAppear:(BOOL)animated {
   [super viewDidAppear:animated];
-  if (!_isAuthenticated) {
+  if (!_remotingService.authentication.user.isAuthenticated) {
     [AppDelegate.instance presentSignInFlow];
     MDCSnackbarMessage* message = [[MDCSnackbarMessage alloc] init];
     message.text = @"Please login.";
     [MDCSnackbarManager showMessage:message];
   } else {
     [_remotingService requestHostListFetch];
   }
 }
 
 - (UIStatusBarStyle)preferredStatusBarStyle {
   return UIStatusBarStyleLightContent;
 }
 
 #pragma mark - Remoting Service Notifications
 
 - (void)hostsDidUpdateNotification:(NSNotification*)notification {
   [_collectionViewController.collectionView reloadData];
 }
 
 - (void)userDidUpdateNotification:(NSNotification*)notification {
-  [self nowAuthenticated:_remotingService.authentication.user.isAuthenticated];
+  [[NSOperationQueue mainQueue] addOperationWithBlock:^{
+    [self
+        nowAuthenticated:_remotingService.authentication.user.isAuthenticated];
+  }];
 }
 
 #pragma mark - RemotingAuthenticationDelegate
 
 - (void)nowAuthenticated:(BOOL)authenticated {
+  _isAuthenticated = authenticated;
   if (authenticated) {
-    MDCSnackbarMessage* message = [[MDCSnackbarMessage alloc] init];
-    message.text = @"Logged In!";
-    [MDCSnackbarManager showMessage:message];
+    _showAccountSignIn = YES;
+    [self presentStatus];
   } else {
     MDCSnackbarMessage* message = [[MDCSnackbarMessage alloc] init];

[Yuwei, 2017/07/06 22:29:45]

If we already presentSignInFlow if the user hasn't signed in then I don't feel
this message is useful... WDYT?

Also I feel like we will need a dedicated "ask you to sign in" view rather than
just popping the sign in flow and force the user to sign in.

Anyway, UX concerns though...

[nicholss, 2017/07/06 22:40:01]

Acknowledged.

-    message.text = @"Not logged in.";
+    message.text = @"Please sign-in.";
     [MDCSnackbarManager showMessage:message];
   }
-  _isAuthenticated = authenticated;
   [_collectionViewController.collectionView reloadData];
 }
 
 #pragma mark - RemotingHostListDelegate
 
 // TODO(nicholss): these need to be a stats change like "none, loading,
 // updated"...
 - (void)hostListUpdated {
   [_collectionViewController.collectionView reloadData];
 }
@@ skipping 62 matching lines @@
   // controls that.
   [_remotingService requestHostListFetch];
 }
 
 - (void)didSelectMenu {
   [AppDelegate.instance showMenuAnimated:YES];
 }
 
 - (void)presentStatus {
   MDCSnackbarMessage* message = [[MDCSnackbarMessage alloc] init];
-  if (_isAuthenticated) {
+  if (_isAuthenticated && _showAccountSignIn) {

[Yuwei, 2017/07/06 22:29:45]

I think we still want to show the message if the user switches account. You can
switch accounts with the SSO menu in internal build.

I kind of feel like it should be some singleton's responsibility (AppDelegate?)
to listen to the event and show the message. That could guarantee the message
will be shown exactly once when the user gets changed.

[nicholss, 2017/07/06 22:40:01]

I think that sounds like a good refactor, pull all the account messaging out of
this class. Adding an item to the list.

+    _showAccountSignIn = NO;
     message.text = [NSString
         stringWithFormat:@"Currently signed in as %@.",
                          _remotingService.authentication.user.userEmail];
     [MDCSnackbarManager showMessage:message];
   }
 }
 
 @end