Show loading spinner on bookmark panel when syncing (iOS)

This CL is the followup fix for CL/569538/ which was reverted because it
caused timeouts of egtests.  This CL made some change on a egtest to
prevent the timeout. Other than that, this CL is identical to CL/569538.

[Patch set 1] is identical to CL/569538.
[Patch set 2] added the fix to prevent egtest timeout.

[Demo videos]
- account with bookmarks:
https://drive.google.com/open?id=0B1O0Z7eoZMuGeTBjVXdla0RPeFE
- account with no bookmarks:
https://drive.google.com/open?id=0B1O0Z7eoZMuGR0pGb2tYUHcyOWM

TEST=Make sure it works when signed in user has no bookmarks or there is any sync error...

Bug: 696893
Change-Id: I92af0f4a1c9f88f6595f8a851ff92f0b5215132b
Reviewed-on: https://chromium-review.googlesource.com/587517
Commit-Queue: Marti Wong <martiw@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Jean-François Geyelin <jif@chromium.org>
Reviewed-by: Eric Noyau <noyau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491181}

ios/chrome/browser/ui/authentication/BUILD.gn

@@ -189,6 +189,7 @@ source_set("eg_tests") {
     "//ios/chrome/browser/signin",
     "//ios/chrome/browser/ui",
     "//ios/chrome/browser/ui/commands",
+    "//ios/chrome/browser/ui/ntp:ntp_controller",
     "//ios/chrome/browser/ui/settings",
     "//ios/chrome/browser/ui/tools_menu",
     "//ios/chrome/test/app:test_support",

ios/chrome/browser/ui/authentication/signin_interaction_controller_egtest.mm

@@ -12,6 +12,7 @@
 #include "ios/chrome/browser/experimental_flags.h"
 #include "ios/chrome/browser/signin/signin_manager_factory.h"
 #import "ios/chrome/browser/ui/commands/open_url_command.h"
+#import "ios/chrome/browser/ui/ntp/new_tab_page_controller.h"
 #import "ios/chrome/browser/ui/settings/accounts_collection_view_controller.h"
 #import "ios/chrome/browser/ui/settings/import_data_collection_view_controller.h"
 #import "ios/chrome/browser/ui/settings/settings_collection_view_controller.h"
@@ -625,7 +626,14 @@ void AssertAuthenticatedIdentityInActiveProfile(ChromeIdentity* identity) {
 
   // Close sign-in screen and Bookmarks.
   TapButtonWithLabelId(IDS_IOS_ACCOUNT_CONSISTENCY_SETUP_SKIP_BUTTON);
-  if (!IsIPadIdiom()) {
+  if (IsIPadIdiom()) {
+    // Switch back to the Home Panel.  This is to prevent Bookmarks Panel, which
+    // has an infinite spinner, from appearing in the coming tests and causing
+    // timeouts.
+    [chrome_test_util::GetCurrentNewTabPageController()
+        selectPanel:NewTabPage::kHomePanel];
+    [[GREYUIThreadExecutor sharedInstance] drainUntilIdle];
+  } else {
     [[EarlGrey selectElementWithMatcher:NavigationBarDoneButton()]
         performAction:grey_tap()];
   }

ios/chrome/browser/ui/bookmarks/BUILD.gn

@@ -123,6 +123,7 @@ source_set("bookmarks") {
     "//ios/chrome/browser/ui/keyboard",
     "//ios/chrome/browser/ui/material_components",
     "//ios/chrome/browser/ui/ntp",
+    "//ios/chrome/browser/ui/sync",
     "//ios/chrome/browser/undo",
     "//ios/public/provider/chrome/browser",
     "//ios/public/provider/chrome/browser/ui",

ios/chrome/browser/ui/bookmarks/bookmark_collection_view.mm

@@ -32,9 +32,11 @@
 #import "ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h"
 #import "ios/chrome/browser/ui/bookmarks/bookmark_collection_cells.h"
 #import "ios/chrome/browser/ui/bookmarks/bookmark_collection_view_background.h"
+#import "ios/chrome/browser/ui/bookmarks/bookmark_home_waiting_view.h"
 #import "ios/chrome/browser/ui/bookmarks/bookmark_promo_cell.h"
 #import "ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.h"
 #import "ios/chrome/browser/ui/bookmarks/bookmark_utils_ios.h"
+#import "ios/chrome/browser/ui/sync/synced_sessions_bridge.h"
 #include "ios/chrome/browser/ui/ui_util.h"
 #import "ios/chrome/browser/ui/uikit_ui_util.h"
 #include "ios/chrome/grit/ios_strings.h"
@@ -74,16 +76,11 @@ CGFloat rowMarginTablet = 24.0;
 CGFloat rowHeight = 48.0;
 // Minimal acceptable favicon size, in points.
 CGFloat minFaviconSizePt = 16;
-
-// Delay in seconds to which the empty background view will be shown when the
-// collection view is empty.
-// This delay should not be too small to let enough time to load bookmarks
-// from network.
-const NSTimeInterval kShowEmptyBookmarksBackgroundRefreshDelay = 1.0;
 }
 
 @interface BookmarkCollectionView ()<BookmarkPromoCellDelegate,
                                      SigninPromoViewConsumer,
+                                     SyncedSessionsObserver,
                                      UICollectionViewDataSource,
                                      UICollectionViewDelegateFlowLayout,
                                      UIGestureRecognizerDelegate> {
@@ -95,6 +92,9 @@ const NSTimeInterval kShowEmptyBookmarksBackgroundRefreshDelay = 1.0;
   // True if the promo is visible.
   BOOL _promoVisible;
 
+  // True if the loading spinner background is visible.
+  BOOL _spinnerVisible;
+
   // Mediator, helper for the sign-in promo view.
   SigninPromoViewMediator* _signinPromoViewMediator;
 
@@ -107,6 +107,9 @@ const NSTimeInterval kShowEmptyBookmarksBackgroundRefreshDelay = 1.0;
   std::map<IntegerPair, base::CancelableTaskTracker::TaskId> _faviconLoadTasks;
   // Task tracker used for async favicon loads.
   base::CancelableTaskTracker _faviconTaskTracker;
+  // Observer to keep track of the signin and syncing status.
+  std::unique_ptr<synced_sessions::SyncedSessionsObserverBridge>
+      _syncedSessionsObserver;
 }
 
 // Redefined to be readwrite.
@@ -222,6 +225,8 @@ const NSTimeInterval kShowEmptyBookmarksBackgroundRefreshDelay = 1.0;
 
     [self setupViews];
     [self updateCollectionView];
+    _syncedSessionsObserver.reset(
+        new synced_sessions::SyncedSessionsObserverBridge(self, _browserState));
   }
   return self;
 }
@@ -899,21 +904,6 @@ const NSTimeInterval kShowEmptyBookmarksBackgroundRefreshDelay = 1.0;
 
 #pragma mark - empty background
 
-// Schedules showing or hiding the empty bookmarks background view if the
-// collection view is empty by calling showEmptyBackgroundIfNeeded after
-// kShowEmptyBookmarksBackgroundRefreshDelay.
-// Multiple call to this method will cancel previous scheduled call to
-// showEmptyBackgroundIfNeeded before scheduling a new one.
-- (void)scheduleEmptyBackgroundVisibilityUpdate {
-  [NSObject cancelPreviousPerformRequestsWithTarget:self
-                                           selector:@selector
-                                           (updateEmptyBackgroundVisibility)
-                                             object:nil];
-  [self performSelector:@selector(updateEmptyBackgroundVisibility)
-             withObject:nil
-             afterDelay:kShowEmptyBookmarksBackgroundRefreshDelay];
-}
-
 - (BOOL)isCollectionViewEmpty {
   BOOL collectionViewIsEmpty = YES;
   const NSInteger numberOfSections = [self numberOfSections];
@@ -926,13 +916,6 @@ const NSTimeInterval kShowEmptyBookmarksBackgroundRefreshDelay = 1.0;
   return collectionViewIsEmpty;
 }
 
-// Shows/hides empty bookmarks background view if the collections view is empty.
-- (void)updateEmptyBackgroundVisibility {
-  const BOOL showEmptyBackground =
-      [self isCollectionViewEmpty] && ![self shouldShowPromoCell];
-  [self setEmptyBackgroundVisible:showEmptyBackground];
-}
-
 // Shows/hides empty bookmarks background view with an animation.
 - (void)setEmptyBackgroundVisible:(BOOL)emptyBackgroundVisible {
   [UIView beginAnimations:@"alpha" context:NULL];
@@ -940,6 +923,55 @@ const NSTimeInterval kShowEmptyBookmarksBackgroundRefreshDelay = 1.0;
   [UIView commitAnimations];
 }
 
+// If the collection view is not empty, hide the empty bookmarks and loading
+// spinner background. If the collection view is empty, shows the loading
+// spinner background (if syncing) or the empty bookmarks background (if not
+// syncing).
+- (void)showEmptyOrLoadingSpinnerBackgroundIfNeeded {
+  const BOOL isEmpty =
+      [self isCollectionViewEmpty] && ![self shouldShowPromoCell];
+  if (!isEmpty) {
+    [self hideLoadingSpinnerBackground];
+    [self hideEmptyBackground];
+    return;
+  }
+  if (_syncedSessionsObserver->IsSyncing()) {
+    [self showLoadingSpinnerBackground];
+    return;
+  }
+  [self showEmptyBackground];
+}
+
+// Shows loading spinner background view
+- (void)showLoadingSpinnerBackground {
+  if (!_spinnerVisible) {
+    BookmarkHomeWaitingView* spinnerView = [[BookmarkHomeWaitingView alloc]
+        initWithFrame:self.collectionView.bounds];
+    [spinnerView startWaiting];
+    self.collectionView.backgroundView = spinnerView;
+    [self setEmptyBackgroundVisible:NO];
+    _spinnerVisible = YES;
+  }
+}
+
+// Hides loading spinner background view
+- (void)hideLoadingSpinnerBackground {
+  if (_spinnerVisible) {
+    self.collectionView.backgroundView = self.emptyCollectionBackgroundView;
+    _spinnerVisible = NO;
+  }
+}
+
+- (void)showEmptyBackground {
+  [self hideLoadingSpinnerBackground];
+  [self setEmptyBackgroundVisible:YES];
+  self.collectionView.backgroundView = self.emptyCollectionBackgroundView;
+}
+
+- (void)hideEmptyBackground {
+  [self setEmptyBackgroundVisible:NO];
+}
+
 #pragma mark - UICollectionViewDataSource
 
 - (NSInteger)collectionView:(UICollectionView*)collectionView
@@ -948,12 +980,7 @@ const NSTimeInterval kShowEmptyBookmarksBackgroundRefreshDelay = 1.0;
       [self numberOfItemsInSection:section];
   const BOOL isCollectionViewEmpty = [self isCollectionViewEmpty];
   self.collectionView.scrollEnabled = !isCollectionViewEmpty;
-  if (isCollectionViewEmpty) {
-    [self scheduleEmptyBackgroundVisibilityUpdate];
-  } else {
-    // Hide empty bookmarks now.
-    [self setEmptyBackgroundVisible:NO];
-  }
+  [self showEmptyOrLoadingSpinnerBackgroundIfNeeded];
   return numberOfItemsInSection;
 }
 
@@ -962,12 +989,7 @@ const NSTimeInterval kShowEmptyBookmarksBackgroundRefreshDelay = 1.0;
   const NSInteger numberOfSections = [self numberOfSections];
   const BOOL collectionViewIsEmpty = 0 == numberOfSections;
   self.collectionView.scrollEnabled = !collectionViewIsEmpty;
-  if (collectionViewIsEmpty) {
-    [self scheduleEmptyBackgroundVisibilityUpdate];
-  } else {
-    // Hide empty bookmarks now.
-    [self setEmptyBackgroundVisible:NO];
-  }
+  [self showEmptyOrLoadingSpinnerBackgroundIfNeeded];
   return numberOfSections;
 }
 
@@ -1284,4 +1306,13 @@ const NSTimeInterval kShowEmptyBookmarksBackgroundRefreshDelay = 1.0;
   return NO;
 }
 
+#pragma mark - Exposed to the SyncedSessionsObserver
+
+- (void)reloadSessions {
+}
+
+- (void)onSyncStateChanged {
+  [self showEmptyOrLoadingSpinnerBackgroundIfNeeded];
+}
+
 @end

ios/chrome/browser/ui/ntp/recent_tabs/BUILD.gn

@@ -21,7 +21,6 @@ source_set("recent_tabs") {
     "//base",
     "//components/browser_sync",
     "//components/sessions",
-    "//components/signin/core/browser",
     "//components/sync",
     "//ios/chrome/app/strings",
     "//ios/chrome/app/theme",
@@ -29,7 +28,6 @@ source_set("recent_tabs") {
     "//ios/chrome/browser/browser_state",
     "//ios/chrome/browser/metrics:metrics_internal",
     "//ios/chrome/browser/sessions",
-    "//ios/chrome/browser/signin",
     "//ios/chrome/browser/sync",
     "//ios/chrome/browser/ui",
     "//ios/chrome/browser/ui/authentication:authentication_arc",

ios/chrome/browser/ui/ntp/recent_tabs/recent_tabs_panel_controller.mm

@@ -8,12 +8,10 @@
 
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/sessions/core/tab_restore_service.h"
-#include "components/signin/core/browser/signin_manager.h"
 #include "components/sync_sessions/open_tabs_ui_delegate.h"
 #include "components/sync_sessions/synced_session.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/sessions/ios_chrome_tab_restore_service_factory.h"
-#include "ios/chrome/browser/signin/signin_manager_factory.h"
 #include "ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.h"
 #include "ios/chrome/browser/sync/sync_setup_service.h"
 #include "ios/chrome/browser/sync/sync_setup_service_factory.h"
@@ -187,9 +185,7 @@
 #pragma mark - Private
 
 - (BOOL)isSignedIn {
-  SigninManager* signin_manager =
-      ios::SigninManagerFactory::GetForBrowserState(_browserState);
-  return signin_manager->IsAuthenticated();
+  return _syncedSessionsObserver->IsSignedIn();
 }
 
 - (BOOL)isSyncTabsEnabled {

ios/chrome/browser/ui/sync/synced_sessions_bridge.h

@@ -45,11 +45,19 @@ class SyncedSessionsObserverBridge : public SyncObserverBridge,
   // Returns true if the first sync cycle that contains session information is
   // completed. Returns false otherwise.
   bool IsFirstSyncCycleCompleted();
+  // Returns true if user is signed in.
+  bool IsSignedIn();
+  // Returns true if it is undergoing the first sync cycle.
+  bool IsSyncing();
+  // Check if the first sync cycle is completed.  This keeps
+  // IsFirstSyncCycleCompleted() and first_sync_cycle_is_completed_ updated.
+  void CheckIfFirstSyncIsCompleted();
 
  private:
   base::WeakNSProtocol<id<SyncedSessionsObserver>> owner_;
   SigninManager* signin_manager_;
   syncer::SyncService* sync_service_;
+  ios::ChromeBrowserState* browser_state_;
   ScopedObserver<SigninManagerBase, SigninManagerBase::Observer>
       signin_manager_observer_;
   // Stores whether the first sync cycle that contains session information is

ios/chrome/browser/ui/sync/synced_sessions_bridge.mm

@@ -10,6 +10,8 @@
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/signin/signin_manager_factory.h"
 #include "ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.h"
+#include "ios/chrome/browser/sync/sync_setup_service.h"
+#include "ios/chrome/browser/sync/sync_setup_service_factory.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -30,9 +32,11 @@ SyncedSessionsObserverBridge::SyncedSessionsObserverBridge(
           ios::SigninManagerFactory::GetForBrowserState(browserState)),
       sync_service_(
           IOSChromeProfileSyncServiceFactory::GetForBrowserState(browserState)),
+      browser_state_(browserState),
       signin_manager_observer_(this),
       first_sync_cycle_is_completed_(false) {
   signin_manager_observer_.Add(signin_manager_);
+  CheckIfFirstSyncIsCompleted();
 }
 
 SyncedSessionsObserverBridge::~SyncedSessionsObserverBridge() {}
@@ -40,15 +44,14 @@ SyncedSessionsObserverBridge::~SyncedSessionsObserverBridge() {}
 #pragma mark - SyncObserverBridge
 
 void SyncedSessionsObserverBridge::OnStateChanged(syncer::SyncService* sync) {
-  if (!signin_manager_->IsAuthenticated())
+  if (!IsSignedIn())
     first_sync_cycle_is_completed_ = false;
   [owner_ onSyncStateChanged];
 }
 
 void SyncedSessionsObserverBridge::OnSyncCycleCompleted(
     syncer::SyncService* sync) {
-  if (sync_service_->GetActiveDataTypes().Has(syncer::SESSIONS))
-    first_sync_cycle_is_completed_ = true;
+  CheckIfFirstSyncIsCompleted();
   [owner_ onSyncStateChanged];
 }
 
@@ -66,6 +69,11 @@ bool SyncedSessionsObserverBridge::IsFirstSyncCycleCompleted() {
   return first_sync_cycle_is_completed_;
 }
 
+void SyncedSessionsObserverBridge::CheckIfFirstSyncIsCompleted() {
+  first_sync_cycle_is_completed_ =
+      sync_service_->GetActiveDataTypes().Has(syncer::SESSIONS);
+}
+
 #pragma mark - SigninManagerBase::Observer
 
 void SyncedSessionsObserverBridge::GoogleSignedOut(
@@ -75,4 +83,24 @@ void SyncedSessionsObserverBridge::GoogleSignedOut(
   [owner_ reloadSessions];
 }
 
+#pragma mark - Signin and syncing status
+
+bool SyncedSessionsObserverBridge::IsSignedIn() {
+  return signin_manager_->IsAuthenticated();
+}
+
+bool SyncedSessionsObserverBridge::IsSyncing() {
+  if (!IsSignedIn())
+    return false;
+
+  SyncSetupService* sync_setup_service =
+      SyncSetupServiceFactory::GetForBrowserState(browser_state_);
+
+  bool sync_enabled = sync_setup_service->IsSyncEnabled();
+  bool no_sync_error = (sync_setup_service->GetSyncServiceState() ==
+                        SyncSetupService::kNoSyncServiceError);
+
+  return sync_enabled && no_sync_error && !IsFirstSyncCycleCompleted();
+}
+
 }  // namespace synced_sessions