MD Settings: simplify animation scheduling

chrome/browser/resources/settings/settings_page/main_page_behavior.js

Index: chrome/browser/resources/settings/settings_page/main_page_behavior.js
diff --git a/chrome/browser/resources/settings/settings_page/main_page_behavior.js b/chrome/browser/resources/settings/settings_page/main_page_behavior.js
index 5e408b2ec9c7e96a2709b7a7133e4003c5d440a2..b1b31d65f94fdb1f079fe57c82c4d301d3aa31a0 100644
--- a/chrome/browser/resources/settings/settings_page/main_page_behavior.js
+++ b/chrome/browser/resources/settings/settings_page/main_page_behavior.js
@@ -42,218 +42,178 @@ var MainPageBehaviorImpl = {
    * @param {!settings.Route} oldRoute
    */
   currentRouteChanged: function(newRoute, oldRoute) {
-    var newRouteIsSubpage = newRoute && newRoute.subpage.length;
-    var oldRouteIsSubpage = oldRoute && oldRoute.subpage.length;
+    // Allow the page to load before expanding the section. TODO(michaelpg):
+    // Time this better when refactoring settings-animated-pages.
+    if (!oldRoute && newRoute.subpage.length)
+      setTimeout(this.tryTransitionToSection_.bind(this));
+    else
+      this.tryTransitionToSection_();
+  },
 
-    if (!oldRoute && newRouteIsSubpage) {
-      // Allow the page to load before expanding the section. TODO(michaelpg):
-      // Time this better when refactoring settings-animated-pages.
-      setTimeout(function() {
-        var section = this.getSection_(newRoute.section);
-        if (section)
-          this.expandSection_(section);
-      }.bind(this));
+  /**
+   * If possible, transitions to the current route's section (by expanding or
+   * scrolling to it). If another transition is running, finishes or cancels
+   * that one, then schedules this function again. This ensures the current
+   * section is quickly shown, without getting the page into a broken state --
+   * if currentRoute changes in between calls, just transition to the new route.
+   * @private
+   */
+  tryTransitionToSection_: function() {
+    var currentRoute = settings.getCurrentRoute();
+    var currentSection = this.getSection_(currentRoute.section);
+
+    // If an animation is already playing, try finishing or canceling it.
+    if (this.currentAnimation_) {
+      this.maybeStopCurrentAnimation_();
+      // Either way, this function will be called again once the current
+      // animation ends.
       return;
     }
 
-    if (newRouteIsSubpage) {
-      if (!oldRouteIsSubpage || newRoute.section != oldRoute.section) {
-        var section = this.getSection_(newRoute.section);
-        if (section)
-          this.expandSection_(section);
-      }
-    } else {
-      if (oldRouteIsSubpage) {
-        var section = this.getSection_(oldRoute.section);
-        if (section)
-          this.collapseSection_(section);
+    var promise;
+    var expandedSection = /** @type {?SettingsSectionElement} */(
+        this.$$('settings-section.expanded'));
+    if (expandedSection) {
+      // If the section shouldn't be expanded, collapse it.
+      if (!currentRoute.subpage.length || expandedSection != currentSection) {
+        promise = this.collapseSection_(expandedSection);
+        // Scroll to the collapsed section. TODO(michaelpg): This can look weird
+        // because the collapse we just scheduled calculated its end target
+        // based on the current scroll position. This bug existed before, and is
+        // fixed in the next patch by making the card position: absolute.
+        if (currentSection)
+          this.scrollToSection_();
       }
-
-      // Scrolls to the section if this main page contains the route's section.
-      if (newRoute && newRoute.section && this.getSection_(newRoute.section))
+    } else if (currentSection) {
+      // Expand the section into a subpage or scroll to it on the main page.
+      if (currentRoute.subpage.length)
+        promise = this.expandSection_(currentSection);
+      else
         this.scrollToSection_();
     }
+
+    // When this animation ends, another may be necessary. Call this function
+    // again after the promise resolves.
+    if (promise)
+      promise.then(this.tryTransitionToSection_.bind(this));
   },
 
   /**
-   * Animates the card in |section|, expanding it to fill the page.
-   * @param {!SettingsSectionElement} section
+   * If the current animation is inconsistent with the current route, stops the

[Dan Beam, 2016/08/09 18:15:19]

stops -> stop?

[michaelpg, 2016/08/09 20:44:03]

figure it should still be indicative, like other function comments

+   * animation by finishing or canceling it so the new route can be animated to.
    * @private
    */
-  expandSection_: function(section) {
-    // If another section's card is expanding, cancel that animation first.
-    var expanding = this.$$('.expanding');
-    if (expanding) {
-      if (expanding == section)
-        return;
-
-      if (this.animations['section']) {
-        // Cancel the animation, then call startExpandSection_.
-        this.cancelAnimation('section', function() {
-          this.startExpandSection_(section);
-        }.bind(this));
-      } else {
-        // The animation must have finished but its promise hasn't resolved yet.
-        // When it resolves, collapse that section's card before expanding
-        // this one.
-        setTimeout(function() {
-          this.collapseSection_(
-              /** @type {!SettingsSectionElement} */(expanding));
-          this.finishAnimation('section', function() {
-            this.startExpandSection_(section);
-          }.bind(this));
-        }.bind(this));
+  maybeStopCurrentAnimation_: function() {
+    var currentRoute = settings.getCurrentRoute();
+    var animatingSection = /** @type {?SettingsSectionElement} */(
+        this.$$('settings-section.expanding, settings-section.collapsing'));

[Dan Beam, 2016/08/09 18:15:19]

i assume it's impossible for these both to exist at once?

[michaelpg, 2016/08/09 20:44:03]

That's the idea, enforced by tryTransitionToSection_ and the animation promises
removing these classes before unsetting currentAnimation_.

+    assert(animatingSection);
+
+    if (animatingSection.classList.contains('expanding')) {
+      // Cancel the animation to go back to the main page if the animating
+      // section shouldn't be expanded.
+      if (animatingSection.section != currentRoute.section ||
+          !currentRoute.subpage.length) {
+        this.currentAnimation_.cancel();
       }
-
+      // Otherwise, let the expand animation continue.
       return;
     }
 
-    if (this.$$('.collapsing') && this.animations['section']) {
-      // Finish the collapse animation before expanding.
-      this.finishAnimation('section', function() {
-        this.startExpandSection_(section);
-      }.bind(this));
+    assert(animatingSection.classList.contains('collapsing'));
+    if (!currentRoute.subpage.length)
+      return;
+
+    // If the collapsing section actually matches the current route's section,
+    // we can just cancel the animation to re-expand the section.
+    if (animatingSection.section == currentRoute.section) {
+      this.currentAnimation_.cancel();
       return;
     }
 
-    this.startExpandSection_(section);
+    // The current route is a subpage, so that section needs to expand.
+    // Immediately finish the current collapse animation so that can happen.
+    this.currentAnimation_.finish();
   },
 
   /**
-   * Helper function to set up and start the expand animation.
+   * Animates the card in |section|, expanding it to fill the page.
    * @param {!SettingsSectionElement} section
+   * @return {!Promise} Resolved when the transition is finished or canceled.
+   * @private
    */
-  startExpandSection_: function(section) {
-    if (!section.canAnimateExpand())
-      return;
-
-    // Freeze the scroller and save its position.
-    this.listScrollTop_ = this.scroller.scrollTop;
+  expandSection_: function(section) {
+    assert(this.scroller);
+    assert(section.canAnimateExpand());
 
-    var scrollerWidth = this.scroller.clientWidth;
-    this.scroller.style.overflow = 'hidden';
-    // Adjust width to compensate for scroller.
-    var scrollbarWidth = this.scroller.clientWidth - scrollerWidth;
-    this.scroller.style.width = 'calc(100% - ' + scrollbarWidth + 'px)';
+    // Save the scroller position before freezing it.
+    this.origScrollTop_ = this.scroller.scrollTop;
+    this.toggleScrolling_(false);
 
-    // Freezes the section's height so its card can be removed from the flow.
+    // Freeze the section's height so its card can be removed from the flow.
     section.setFrozen(true);
 
-    // Expand the section's card to fill the parent.
-    var animationPromise = this.playExpandSection_(section);
+    this.currentAnimation_ = section.animateExpand(this.scroller);
+    var promise = this.currentAnimation_ ?
+        this.currentAnimation_.finished : Promise.resolve();
 
-    animationPromise.then(function() {
+    var finished;
+    return promise.then(function() {
       this.scroller.scrollTop = 0;
       this.toggleOtherSectionsHidden_(section.section, true);
-    }.bind(this), function() {
-      // Animation was canceled; restore the section.
-      section.setFrozen(false);
-    }.bind(this)).then(function() {
-      this.scroller.style.overflow = '';
-      this.scroller.style.width = '';
-    }.bind(this));
-  },
-
-  /**
-   * Expands the card in |section| to fill the page.
-   * @param {!SettingsSectionElement} section
-   * @return {!Promise}
-   * @private
-   */
-  playExpandSection_: function(section) {
-    // We must be attached.
-    assert(this.scroller);
-
-    var promise;
-    var animationConfig = section.animateExpand(this.scroller);
-    if (animationConfig) {
-      promise = this.animateElement('section', animationConfig.card,
-          animationConfig.keyframes, animationConfig.options);
-    } else {
-      promise = Promise.resolve();
-    }
-
-    var finished;
-    promise.then(function() {
       finished = true;
-      this.style.margin = 'auto';
     }.bind(this), function() {
-      // The animation was canceled; catch the error and continue.
+      // The animation was canceled; restore the section.
+      section.setFrozen(false);
       finished = false;
     }).then(function() {
       section.cleanUpAnimateExpand(finished);
-    });
-
-    return promise;
+      this.toggleScrolling_(true);
+      this.currentAnimation_ = null;
+    }.bind(this));
   },
 
   /**
    * Animates the card in |section|, collapsing it back into its section.
    * @param {!SettingsSectionElement} section
+   * @return {!Promise} Resolved when the transition is finished or canceled.
    * @private
    */
   collapseSection_: function(section) {
-    // If the section's card is still expanding, cancel the expand animation.
-    if (section.classList.contains('expanding')) {
-      if (this.animations['section']) {
-        this.cancelAnimation('section');
-      } else {
-        // The animation must have finished but its promise hasn't finished
-        // resolving; try again asynchronously.
-        this.async(function() {
-          this.collapseSection_(section);
-        });
-      }
-      return;
-    }
-
-    if (!section.canAnimateCollapse())
-      return;
+    assert(this.scroller);
+    assert(section.canAnimateCollapse());
 
     this.toggleOtherSectionsHidden_(section.section, false);
+    this.toggleScrolling_(false);
 
-    var scrollerWidth = this.scroller.clientWidth;
-    this.scroller.style.overflow = 'hidden';
-    // Adjust width to compensate for scroller.
-    var scrollbarWidth = this.scroller.clientWidth - scrollerWidth;
-    this.scroller.style.width = 'calc(100% - ' + scrollbarWidth + 'px)';
+    this.currentAnimation_ =
+        section.animateCollapse(this.scroller, this.origScrollTop_);
+    var promise = this.currentAnimation_ ?
+        this.currentAnimation_.finished : Promise.resolve();
 
-    this.playCollapseSection_(section).then(function() {
+    return promise.then(function() {
+      section.cleanUpAnimateCollapse(true);
+    }, function() {
+      section.cleanUpAnimateCollapse(false);
+    }).then(function() {
       section.setFrozen(false);
-      this.scroller.style.overflow = '';
-      this.scroller.style.width = '';
       section.classList.remove('collapsing');
+      this.toggleScrolling_(true);
+      this.currentAnimation_ = null;
     }.bind(this));
   },
 
   /**
-   * Collapses the card in |section| back to its normal position.
-   * @param {!SettingsSectionElement} section
-   * @return {!Promise}
+   * Hides or unhides the sections not being expanded.
+   * @param {string} sectionName The section to keep visible.
+   * @param {boolean} hidden Whether the sections should be hidden.
    * @private
    */
-  playCollapseSection_: function(section) {
-    // We must be attached.
-    assert(this.scroller);
-
-    this.style.margin = '';
-
-    var promise;
-    var animationConfig =
-        section.animateCollapse(this.scroller, this.listScrollTop_);
-    if (animationConfig) {
-      promise = this.animateElement('section', animationConfig.card,
-          animationConfig.keyframes, animationConfig.options);
-    } else {
-      promise = Promise.resolve();
-    }
-
-    promise.then(function() {
-      section.cleanUpAnimateCollapse(true);
-    }, function() {
-      section.cleanUpAnimateCollapse(false);
-    });
-    return promise;
+  toggleOtherSectionsHidden_: function(sectionName, hidden) {
+    var sections = Polymer.dom(this.root).querySelectorAll(
+        'settings-section');
+    for (var section of sections)
+      section.hidden = hidden && (section.section != sectionName);
   },
 
   /** @private */
@@ -265,38 +225,47 @@ var MainPageBehaviorImpl = {
         function() {
           // If the current section changes while we are waiting for the page to
           // be ready, scroll to the newest requested section.
-          this.getSection_(settings.getCurrentRoute().section).scrollIntoView();
+          var section = this.getSection_(settings.getCurrentRoute().section);
+          if (section)
+            section.scrollIntoView();
         }.bind(this));
   },
 
   /**
-   * Hides or unhides the sections not being expanded.
-   * @param {string} sectionName The section to keep visible.
-   * @param {boolean} hidden Whether the sections should be hidden.
-   * @private
-   */
-  toggleOtherSectionsHidden_: function(sectionName, hidden) {
-    var sections = Polymer.dom(this.root).querySelectorAll(
-        'settings-section');
-    for (var section of sections)
-      section.hidden = hidden && (section.section != sectionName);
-  },
-
-  /**
    * Helper function to get a section from the local DOM.
    * @param {string} section Section name of the element to get.
    * @return {?SettingsSectionElement}
    * @private
    */
   getSection_: function(section) {
+    if (!section)
+      return null;
     return /** @type {?SettingsSectionElement} */(
         this.$$('[section=' + section + ']'));
   },
+
+  /**
+   * Enables or disables user scrolling, via overscroll: hidden. Room for the
+   * hidden scrollbar is added to prevent the page width from changing back and
+   * forth.
+   * @param {boolean} enabled
+   * @private
+   */
+  toggleScrolling_: function(enabled) {
+    if (enabled) {
+      this.scroller.style.overflow = '';
+      this.scroller.style.width = '';
+    } else {
+      var scrollerWidth = this.scroller.clientWidth;
+      this.scroller.style.overflow = 'hidden';
+      var scrollbarWidth = this.scroller.clientWidth - scrollerWidth;
+      this.scroller.style.width = 'calc(100% - ' + scrollbarWidth + 'px)';
+    }
+  }
 };
 
 /** @polymerBehavior */
 var MainPageBehavior = [
   settings.RouteObserverBehavior,
-  TransitionBehavior,
   MainPageBehaviorImpl,
 ];