Fixed resizing animation for extension options.

chrome/browser/resources/extensions/extension_options_overlay.js

 // Copyright 2014 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.
 
 cr.define('extensions', function() {
   'use strict';
 
   /**
    * The ExtensionOptionsOverlay will show an extension's options page using
    * an <extensionoptions> element.
@@ skipping 88 matching lines @@
       this.setVisible_(true);
 
       var extensionoptions = new window.ExtensionOptions();
       extensionoptions.extension = extensionId;
 
       // The <extensionoptions> content's size needs to be restricted to the
       // bounds of the overlay window. The overlay gives a minWidth and
       // maxHeight, but the maxHeight does not include our header height (title
       // and close button), so we need to subtract that to get the maxHeight
       // for the extension options.
       var maxHeight = parseInt(overlay.style.maxHeight, 10) -

[not at google - send to devlin, 2015/03/18 15:35:20]

This is funny, I wonder why it's overlay.style.maxHeight rather than
overlayStyle.maxHeight? I wonder if it matters? It should probably be
overlayStyle.maxHeight.

[paulmeyer, 2015/03/18 17:37:48]

Done.

                       overlayHeader.offsetHeight;
       var minWidth = parseInt(overlayStyle.minWidth, 10);
 
       extensionoptions.onclose = function() {
         cr.dispatchSimpleEvent($('overlay'), 'cancelOverlay');
       }.bind(this);
 
       // Track the current animation (used to grow/shrink the overlay content),
       // if any. Preferred size changes can fire more rapidly than the
       // animation speed, and multiple animations running at the same time has
       // undesirable effects.
       var animation = null;
 
       /**
        * Resize the overlay if the <extensionoptions> changes preferred size.
        * @param {{width: number, height: number}} evt
        */
       extensionoptions.onpreferredsizechanged = function(evt) {
+        // |overlayStyle.height| includes the header height, so it needs to be
+        // subtracted to get the true old preferred height.

[not at google - send to devlin, 2015/03/18 15:35:20]

And I wonder if this should use the width/height of overlayGuest rather than the
width/height of the overlay (where the width is the same of course)?

[paulmeyer, 2015/03/18 17:37:48]

I think the reason that overlayGuest isn't used here is because its width and
height are undefined the first time this handler is called. Do you still think
its worth using them instead here (and initialize them as 0/0 somewhere else
ahead of time)?

[not at google - send to devlin, 2015/03/18 19:52:06]

Ah, that could be it.

Once again rendering these offscreen then detach/attaching them to render at the
initial correct height would solve this, but well outside the scope of this
change.

Actually - what do they start off as in this case? Is it 0,0?

Btw can we rename these to oldGuestWidth and oldGuestHeight?

[paulmeyer, 2015/03/18 21:41:46]

Yes, 0,0.

I agree, those names are more meaningful.

Done.

         var oldWidth = parseInt(overlayStyle.width, 10);
-        var oldHeight = parseInt(overlayStyle.height, 10);
+        var oldHeight =
+            parseInt(overlayStyle.height, 10) - overlayHeader.offsetHeight;
         // The overlay must be slightly larger than the extension options to
         // avoid creating scrollbars.
         // TODO(paulmeyer): This shouldn't be necessary, but the preferred size
         // (coming from Blink) seems to be too small for some zoom levels. The
         // 2-pixel addition should be removed once this problem is investigated
         // and corrected.
         var newWidth = Math.max(evt.width + 2, minWidth);
         var newHeight = Math.min(evt.height + 2, maxHeight);
 
         // animationTime is the amount of time in ms that will be used to resize
         // the overlay. It is calculated by multiplying the pythagorean distance
         // between old and the new size (in px) with a constant speed of
         // 0.25 ms/px.
         var loading = document.documentElement.classList.contains('loading');
         var animationTime = loading ? 0 :
             0.25 * Math.sqrt(Math.pow(newWidth - oldWidth, 2) +
                              Math.pow(newHeight - oldHeight, 2));
 
         if (animation)
           animation.cancel();
 
+        // The header height must be added to the (old and new) preferred
+        // heights to get the full overlay heights.
         animation = overlay.animate([
-          {width: oldWidth + 'px', height: oldHeight + 'px'},
-          {width: newWidth + 'px', height: newHeight + 'px'}
+          {width: oldWidth + 'px',
+           height: (oldHeight + overlayHeader.offsetHeight) + 'px'},
+          {width: newWidth + 'px',
+           height: (newHeight + overlayHeader.offsetHeight) + 'px'}
         ], {
           duration: animationTime,
           delay: 0
         });
 
         animation.onfinish = function(e) {
           animation = null;
 
           // The <extensionoptions> element is ready to place back in the
           // overlay. Make sure that it's sized to take up the full width/height
@@ skipping 35 matching lines @@
           /** @type {HTMLDivElement} */($('extension-options-overlay')) :
           null);
     }
   };
 
   // Export
   return {
     ExtensionOptionsOverlay: ExtensionOptionsOverlay
   };
 });