Commit 144eba44 authored by Philip Rogers's avatar Philip Rogers Committed by Commit Bot

Add hysteresis to extensions options dialog preferred size logic

The md extension options dialog has an inner frame with the options page
contents. The dialog is sized based on the preferred size of the options
page contents using preferred size change events. The options page
contents's size is also dependent on the dialog size, so resizing the
dialog can result in a preferred size change event which then resizes
the dialog size again. Because the options dialog and the options page
contents are on different threads, this size update logic can infinitely
oscillate between two sizes.

This bug became more prominent with two recent changes:
https://crrev.com/581383 - made preferred size changes synchronous
https://crrev.com/582708 - moved preferred size change events from the
middle of a frame to the end of a frame.

This patch adds a delay between getting the preferred size update and
changing the dialog size, fixing the options dialog size oscillation.
This mimics the asynchronous preferred size updates that were done
before https://crrev.com/581383.

Test: manually tested using 4 custom extension options pages (short
quirksmode, tall quirksmode, short html5, and tall html5).

Bug: 882835
Change-Id: I97bd61c3137ae851d855637aed75029bce1e5404
Reviewed-on: https://chromium-review.googlesource.com/c/1259040
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596359}
parent 0a9bf07f
......@@ -84,7 +84,13 @@ cr.define('extensions', function() {
preferredSize = e;
if (!this.$.dialog.open)
this.$.dialog.showModal();
this.updateDialogSize_(preferredSize.width, preferredSize.height);
// Updating the dialog size can result in a preferred size change, so
// wait until request animation frame fires before updating the dialog
// size. This hysteresis prevents the preferred size from oscillating
// (see: https://crbug.com/882835).
requestAnimationFrame(() => {
this.updateDialogSize_(preferredSize.width, preferredSize.height);
});
};
this.boundResizeListener_ = () => {
......
......@@ -42,19 +42,23 @@ cr.define('extension_options_dialog_tests', function() {
optionsDialog.show(data);
return test_util.eventToPromise('cr-dialog-open', optionsDialog)
.then(function() {
assertTrue(isDialogVisible());
// The dialog size is set asynchronously (see onpreferredsizechanged
// in options_dialog.js) so wait one frame.
requestAnimationFrame(function() {
assertTrue(isDialogVisible());
const dialogElement = optionsDialog.$.dialog.getNative();
const rect = dialogElement.getBoundingClientRect();
assertGE(rect.width, extensions.OptionsDialogMinWidth);
assertLE(rect.height, extensions.OptionsDialogMaxHeight);
// This is the header height with default font size.
assertGE(rect.height, 68);
const dialogElement = optionsDialog.$.dialog.getNative();
const rect = dialogElement.getBoundingClientRect();
assertGE(rect.width, extensions.OptionsDialogMinWidth);
assertLE(rect.height, extensions.OptionsDialogMaxHeight);
// This is the header height with default font size.
assertGE(rect.height, 68);
assertEquals(
data.name,
assert(optionsDialog.$$('#icon-and-name-wrapper span'))
.textContent.trim());
assertEquals(
data.name,
assert(optionsDialog.$$('#icon-and-name-wrapper span'))
.textContent.trim());
});
});
});
});
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment