Fix potential flakiness in Files.app test

This patch makes the banner & focus code robust with promise. And make sure the banner existences before test runs.

BUG=478640
TEST=run TabindexFocusDownloads/FileManagerBrowserTest.Test/0 100 times

Review URL: https://codereview.chromium.org/1105333003

Cr-Commit-Position: refs/heads/master@{#327255}

ui/file_manager/file_manager/foreground/js/file_manager.js

@@ -598,6 +598,9 @@ FileManager.prototype = /** @struct */ {
     this.initializeQueue_.add(
         this.initFileSystemUI_.bind(this),
         ['initAdditionalUI', 'initSettings'], 'initFileSystemUI');
+    this.initializeQueue_.add(
+        this.initUIFocus_.bind(this),
+        ['initAdditionalUI', 'initFileSystemUI'], 'initUIFocus');
 
     // Run again just in case if all pending closures have completed and the
     // queue has stopped and monitor the completion.
@@ -803,6 +806,17 @@ FileManager.prototype = /** @struct */ {
     callback();
   };
 
+  /**
+   * One-time initialization of focus. This should run at the last of UI
+   *  initialization.
+   *
+   * @private
+   */
+  FileManager.prototype.initUIFocus_ = function(callback) {
+    this.ui_.initUIFocus();
+    callback();
+  };
+
   /**
    * One-time initialization of import history observer. Provides
    * the glue that updates the UI when history changes.

ui/file_manager/file_manager/foreground/js/ui/banners.js

@@ -42,20 +42,24 @@ function Banners(
   chrome.storage.onChanged.addListener(this.onStorageChange_.bind(this));
   this.welcomeHeaderCounter_ = WELCOME_HEADER_COUNTER_LIMIT;
   this.warningDismissedCounter_ = 0;
-  chrome.storage.local.get(
-      [WELCOME_HEADER_COUNTER_KEY, WARNING_DISMISSED_KEY],
-      function(values) {
-        this.welcomeHeaderCounter_ =
-            parseInt(values[WELCOME_HEADER_COUNTER_KEY], 10) || 0;
-        this.warningDismissedCounter_ =
-            parseInt(values[WARNING_DISMISSED_KEY], 10) || 0;
-
-        // If it's in test, override the counter to show the header by force.
-        if (window.IN_TEST) {
-          this.welcomeHeaderCounter_ = 0;
-          this.warningDismissedCounter_ = 0;
-        }
-      }.bind(this));
+
+  this.ready_ = new Promise(function(resolve, reject) {
+    chrome.storage.local.get(
+        [WELCOME_HEADER_COUNTER_KEY, WARNING_DISMISSED_KEY],
+        function(values) {
+          this.welcomeHeaderCounter_ =
+              parseInt(values[WELCOME_HEADER_COUNTER_KEY], 10) || 0;
+          this.warningDismissedCounter_ =
+              parseInt(values[WARNING_DISMISSED_KEY], 10) || 0;
+
+          // If it's in test, override the counter to show the header by force.
+          if (chrome.test) {
+            this.welcomeHeaderCounter_ = 0;
+            this.warningDismissedCounter_ = 0;
+          }
+          resolve();
+        }.bind(this));
+  }.bind(this));
 
   // Authentication failed banner.
   this.authFailedBanner_ =
@@ -301,73 +305,76 @@ Banners.prototype.closeWelcomeBanner_ = function() {
  * @private
  */
 Banners.prototype.checkSpaceAndMaybeShowWelcomeBanner_ = function() {
-  if (!this.isOnCurrentProfileDrive()) {
-    // We are not on the drive file system. Do not show (close) the welcome
-    // banner.
-    this.cleanupWelcomeBanner_();
-    this.previousDirWasOnDrive_ = false;
-    return;
-  }
+  this.ready_.then(function() {
+    if (!this.isOnCurrentProfileDrive()) {
+      // We are not on the drive file system. Do not show (close) the welcome
+      // banner.
+      this.cleanupWelcomeBanner_();
+      this.previousDirWasOnDrive_ = false;
+      return;
+    }
 
-  var driveVolume = this.volumeManager_.getCurrentProfileVolumeInfo(
-      VolumeManagerCommon.VolumeType.DRIVE);
-  if (this.welcomeHeaderCounter_ >= WELCOME_HEADER_COUNTER_LIMIT ||
-      !driveVolume || driveVolume.error) {
-    // The banner is already shown enough times or the drive FS is not mounted.
-    // So, do nothing here.
-    return;
-  }
+    var driveVolume = this.volumeManager_.getCurrentProfileVolumeInfo(
+        VolumeManagerCommon.VolumeType.DRIVE);
+    if (this.welcomeHeaderCounter_ >= WELCOME_HEADER_COUNTER_LIMIT ||
+        !driveVolume || driveVolume.error) {
+      // The banner is already shown enough times or the drive FS is not
+      // mounted. So, do nothing here.
+      return;
+    }
 
-  if (!this.showOffers_ || !this.showWelcome_) {
-    // Because it is not necessary to show the offer, set
-    // |usePromoWelcomeBanner_| false here. Note that it probably should be able
-    // to do this in the constructor, but there remains non-trivial path,
-    // which may be causes |usePromoWelcomeBanner_| == true's behavior even
-    // if |showOffers_| is false.
-    // TODO(hidehiko): Make sure if it is expected or not, and simplify
-    // |showOffers_| if possible.
-    this.usePromoWelcomeBanner_ = false;
-  }
+    if (!this.showOffers_ || !this.showWelcome_) {
+      // Because it is not necessary to show the offer, set
+      // |usePromoWelcomeBanner_| false here. Note that it probably should be
+      // able to do this in the constructor, but there remains non-trivial path,
+      // which may be causes |usePromoWelcomeBanner_| == true's behavior even
+      // if |showOffers_| is false.
+      // TODO(hidehiko): Make sure if it is expected or not, and simplify
+      // |showOffers_| if possible.
+      this.usePromoWelcomeBanner_ = false;
+    }
 
-  // Perform asynchronous tasks in parallel.
-  var group = new AsyncUtil.Group();
+    // Choose the offer basing on the board name. The default one is 100 GB.
+    var offerSize = 100;  // In GB.
+    var offerServiceId = 'drive.cros.echo.1';
 
-  // Choose the offer basing on the board name. The default one is 100 GB.
-  var offerSize = 100;  // In GB.
-  var offerServiceId = 'drive.cros.echo.1';
+    if (util.boardIs('link')) {
+      offerSize = 1024;  // 1 TB.
+      offerServiceId = 'drive.cros.echo.2';
+    }
 
-  if (util.boardIs('link')) {
-    offerSize = 1024;  // 1 TB.
-    offerServiceId = 'drive.cros.echo.2';
-  }
+    // Perform asynchronous tasks in parallel.
+    var group = new AsyncUtil.Group();
 
-  // If the offer has been checked, then do not show the promo anymore.
-  group.add(function(onCompleted) {
-    chrome.echoPrivate.getOfferInfo(offerServiceId, function(offerInfo) {
-      // If the offer has not been checked, then an error is raised.
-      if (!chrome.runtime.lastError)
-        this.usePromoWelcomeBanner_ = false;
-      onCompleted();
+    // If the offer has been checked, then do not show the promo anymore.
+    group.add(function(onCompleted) {
+      chrome.echoPrivate.getOfferInfo(offerServiceId, function(offerInfo) {
+        // If the offer has not been checked, then an error is raised.
+        if (!chrome.runtime.lastError)
+          this.usePromoWelcomeBanner_ = false;
+        onCompleted();
+      }.bind(this));
     }.bind(this));
-  }.bind(this));
 
-  if (this.usePromoWelcomeBanner_) {
-    // getSizeStats for Drive file system accesses to the server, so we should
-    // minimize the invocation.
     group.add(function(onCompleted) {
-      // Current directory must be set, since this code is called after
-      // scanning is completed. However, the volumeInfo may be gone.
-      chrome.fileManagerPrivate.getSizeStats(
-          driveVolume.volumeId,
-          function(result) {
-            if (result && result.totalSize >= offerSize * 1024 * 1024 * 1024)
-              this.usePromoWelcomeBanner_ = false;
-            onCompleted();
-          }.bind(this));
+      if (this.usePromoWelcomeBanner_) {
+        // getSizeStats for Drive file system accesses to the server, so we
+        // should minimize the invocation.
+
+        // Current directory must be set, since this code is called after
+        // scanning is completed. However, the volumeInfo may be gone.
+        chrome.fileManagerPrivate.getSizeStats(
+            driveVolume.volumeId,
+            function(result) {
+              if (result && result.totalSize >= offerSize * 1024 * 1024 * 1024)
+                this.usePromoWelcomeBanner_ = false;
+              onCompleted();
+            }.bind(this));
+      }
     }.bind(this));
-  }
 
-  group.run(this.maybeShowWelcomeBanner_.bind(this));
+    group.run(this.maybeShowWelcomeBanner_.bind(this));
+  }.bind(this));
 };
 
 /**
@@ -376,27 +383,29 @@ Banners.prototype.checkSpaceAndMaybeShowWelcomeBanner_ = function() {
  * @private
  */
 Banners.prototype.maybeShowWelcomeBanner_ = function() {
-  if (this.directoryModel_.getFileList().length == 0 &&
-      this.welcomeHeaderCounter_ == 0) {
-    // Only show the full page banner if the header banner was never shown.
-    // Do not increment the counter.
-    // The timeout below is required because sometimes another
-    // 'rescan-completed' event arrives shortly with non-empty file list.
-    setTimeout(function() {
-      if (this.isOnCurrentProfileDrive() && this.welcomeHeaderCounter_ == 0) {
-        this.prepareAndShowWelcomeBanner_('page', 'DRIVE_WELCOME_TEXT_LONG');
+  this.ready_.then(function() {
+    if (this.directoryModel_.getFileList().length == 0 &&
+        this.welcomeHeaderCounter_ == 0) {
+      // Only show the full page banner if the header banner was never shown.
+      // Do not increment the counter.
+      // The timeout below is required because sometimes another
+      // 'rescan-completed' event arrives shortly with non-empty file list.
+      setTimeout(function() {
+        if (this.isOnCurrentProfileDrive() && this.welcomeHeaderCounter_ == 0) {
+          this.prepareAndShowWelcomeBanner_('page', 'DRIVE_WELCOME_TEXT_LONG');
+        }
+      }.bind(this), 2000);
+    } else {
+      // We do not want to increment the counter when the user navigates
+      // between different directories on Drive, but we increment the counter
+      // once anyway to prevent the full page banner from showing.
+      if (!this.previousDirWasOnDrive_ || this.welcomeHeaderCounter_ == 0) {
+        this.setWelcomeHeaderCounter_(this.welcomeHeaderCounter_ + 1);
+        this.prepareAndShowWelcomeBanner_('header', 'DRIVE_WELCOME_TEXT_SHORT');
       }
-    }.bind(this), 2000);
-  } else {
-    // We do not want to increment the counter when the user navigates
-    // between different directories on Drive, but we increment the counter
-    // once anyway to prevent the full page banner from showing.
-    if (!this.previousDirWasOnDrive_ || this.welcomeHeaderCounter_ == 0) {
-      this.setWelcomeHeaderCounter_(this.welcomeHeaderCounter_ + 1);
-      this.prepareAndShowWelcomeBanner_('header', 'DRIVE_WELCOME_TEXT_SHORT');
     }
-  }
-  this.previousDirWasOnDrive_ = true;
+    this.previousDirWasOnDrive_ = true;
+  }.bind(this));
 };
 
 /**

ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js

@@ -314,23 +314,24 @@ FileManagerUI.prototype.initAdditionalUI = function(
 
   // Add handlers.
   document.defaultView.addEventListener('resize', this.relayout.bind(this));
+};
 
+/**
+ * Initializes the focus.
+ */
+FileManagerUI.prototype.initUIFocus = function() {
   // Set the initial focus. When there is no focus, the active element is the
   // <body>.
-  setTimeout(function() {
-    if (document.activeElement === document.body) {
-      var targetElement = null;
-      if (this.dialogType_ == DialogType.SELECT_SAVEAS_FILE) {
-        targetElement = this.dialogFooter.filenameInput;
-      } else if (this.listContainer.currentListType !=
-                 ListContainer.ListType.UNINITIALIZED) {
-        targetElement = this.listContainer.currentList;
-      }
-
-      if (targetElement)
-        targetElement.focus();
-    }
-  }.bind(this), 0);
+  var targetElement = null;
+  if (this.dialogType_ == DialogType.SELECT_SAVEAS_FILE) {
+    targetElement = this.dialogFooter.filenameInput;
+  } else if (this.listContainer.currentListType !=
+             ListContainer.ListType.UNINITIALIZED) {
+    targetElement = this.listContainer.currentList;
+  }
+
+  if (targetElement)
+    targetElement.focus();
 };
 
 /**

ui/file_manager/integration_tests/file_manager/tab_index.js

@@ -66,9 +66,12 @@ testcase.tabindexFocus = function() {
       appId = inAppId;
       remoteCall.waitForElement(appId, ['#file-list:focus']).then(this.next);
     },
-    // Press the Tab key.
+    function(element) {
+      remoteCall.waitForElement(appId, ['#drive-welcome-link']).then(this.next);
+    },
     function(element) {
       remoteCall.callRemoteTestUtil('getActiveElement', appId, [], this.next);
+    // Press the Tab key.
     }, function(element) {
       chrome.test.assertEq('list', element.attributes['class']);
       remoteCall.checkNextTabFocus(appId, 'search-button').then(this.next);
@@ -150,7 +153,9 @@ testcase.tabindexFocusDirectorySelected = function() {
       appId = inAppId;
       remoteCall.waitForElement(appId, ['#file-list:focus']).then(this.next);
     },
-    // Press the Tab key.
+    function(element) {
+      remoteCall.waitForElement(appId, ['#drive-welcome-link']).then(this.next);
+    },
     function(element) {
       remoteCall.callRemoteTestUtil('getActiveElement', appId, [], this.next);
     }, function(element) {
@@ -158,6 +163,7 @@ testcase.tabindexFocusDirectorySelected = function() {
       // Select the directory named 'photos'.
       remoteCall.callRemoteTestUtil(
           'selectFile', appId, ['photos']).then(this.next);
+    // Press the Tab key.
     }, function(result) {
       chrome.test.assertTrue(result);
       remoteCall.checkNextTabFocus(appId, 'share-button').then(this.next);