Fix scroll regression when specifying an extension id.

chrome/browser/ui/webui/extensions/extension_settings_browsertest.js

Index: chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
diff --git a/chrome/browser/ui/webui/extensions/extension_settings_browsertest.js b/chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
index f96ed80fa3bb9b4052bf16d6ba11236aab89829d..6b4f70ac549183e7d5b9fde60f6c397698204d28 100644
--- a/chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
+++ b/chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
@@ -29,6 +29,9 @@ ExtensionSettingsWebUITest.prototype = {
   __proto__: testing.Test.prototype,
 
   /** @override */
+  isAsync: true,
+
+  /** @override */
   runAccessibilityChecks: true,
 
   /** @override */
@@ -43,17 +46,119 @@ ExtensionSettingsWebUITest.prototype = {
 
   /** @override */
   typedefCppFixture: 'ExtensionSettingsUIBrowserTest',
+
+  /** @override */
+  setUp: function() {
+    // Make all transitions take 0ms for testing purposes.
+    var noTransitionStyle = document.createElement('style');
+    noTransitionStyle.textContent =
+        '* {' +
+        '  -webkit-transition-duration: 0ms !important;' +
+        '  -webkit-transition-delay: 0ms !important;' +
+        '}';
+    document.querySelector('head').appendChild(noTransitionStyle);
+  },
+
+  /**
+   * Holds an array of steps that should happen in order during a test.
+   * The last step should be |testDone|.
+   * @protected {Array<!Function>}
+   * */
+  steps: [],
+
+  /**
+   * Advances to the next step in the test. Every step should call this.
+   * @protected
+   * */
+  nextStep: function() {
+    assertTrue(this.steps.length > 0);
+    this.steps.shift().call(this);
+  },
+
+  /**
+   * Will wait for the page to load before calling the next step. This should be
+   * the first step in every test.
+   * @protected
+   * */
+  waitForPageLoad: function() {
+    assertEquals(this.browsePreload, document.location.href);
+    var extensionList = getRequiredElement('extension-settings-list');
+    extensionList.loadFinished.then(this.nextStep.bind(this));
+  },
+
+  /** @protected */
+  verifyDeveloperModeWorks: function() {
+    var extensionSettings = getRequiredElement('extension-settings');
+    assertFalse(extensionSettings.classList.contains('dev-mode'));
+    $('toggle-dev-on').click();
+    assertTrue(extensionSettings.classList.contains('dev-mode'));
+    chrome.developerPrivate.getProfileConfiguration(function(profileInfo) {
+      assertTrue(profileInfo.inDeveloperMode);
+
+      // A 0ms timeout is necessary so that all the transitions can finish.
+      window.setTimeout(function() {

[Dan Beam, 2015/04/22 18:44:55]

nit:
  window.setTimeout(this.nextSetup.bind(this), 0);

[hcarmona, 2015/04/22 20:44:45]

Done.

+        this.nextStep(this);

[Dan Beam, 2015/04/22 18:44:55]

this.nextStep();

[hcarmona, 2015/04/22 20:44:45]

Acknowledged.

+      }.bind(this), 0);
+    }.bind(this));
+  },
+
+  /** @protected */
+  testDeveloperMode: function() {
+    var next = this.nextStep.bind(this);
+    var checkDevModeIsOff = function() {
+      chrome.developerPrivate.getProfileConfiguration(function(profileInfo) {
+        assertFalse(profileInfo.inDeveloperMode);
+        next();
+      });
+    };
+    this.steps = [this.waitForPageLoad,
+                  checkDevModeIsOff,
+                  this.verifyDeveloperModeWorks,
+                  testDone];
+    this.nextStep();
+  }
 };
 
-TEST_F('ExtensionSettingsWebUITest', 'testChromeSendHandled', function() {
-  assertEquals(this.browsePreload, document.location.href);
+// Verify that developer mode doesn't change behavior when the number of
+// extensions changes.
+TEST_F('ExtensionSettingsWebUITest', 'testDeveloperModeNoExtensions',
+       function() {
+  this.testDeveloperMode();
+});
+
+TEST_F('ExtensionSettingsWebUITest', 'testEmptyExtensionList', function() {
+  var verifyListIsHiddenAndEmpty = function() {
+    var listWrapper = $('extension-list-wrapper');
+    var noExtensionMessage = $('no-extensions');
+    var list = $('extension-settings-list');
+
+    assertTrue(listWrapper.hidden);

[Dan Beam, 2015/04/22 18:44:54]

nit: inline variables only used once, e.g.

  assertTrue($('extension-list-wrapper').hidden);

[hcarmona, 2015/04/22 20:44:45]

Done.

+    assertFalse(noExtensionMessage.hidden);
+    assertEquals(0, list.childNodes.length);
+    this.nextStep();
+  };
+
+  this.steps = [this.waitForPageLoad,
+                verifyListIsHiddenAndEmpty,
+                testDone];
+  this.nextStep();
+});
 
-  // This dialog should be hidden at first.
-  assertFalse($('pack-extension-overlay').classList.contains('showing'));
+TEST_F('ExtensionSettingsWebUITest', 'testChromeSendHandled', function() {
+  var testPackExtenion = function() {
+    // This dialog should be hidden at first.
+    assertFalse($('pack-extension-overlay').classList.contains('showing'));
+
+    // Show the dialog, which triggers a chrome.send() for metrics purposes.
+    cr.dispatchSimpleEvent($('pack-extension'), 'click');
+    assertTrue($('pack-extension-overlay').classList.contains('showing'));
+    this.nextStep();
+  };
 
-  // Show the dialog, which triggers a chrome.send() for metrics purposes.
-  cr.dispatchSimpleEvent($('pack-extension'), 'click');
-  assertTrue($('pack-extension-overlay').classList.contains('showing'));
+  this.steps = [this.waitForPageLoad,
+                testPackExtenion,
+                testDone];
+  this.nextStep();
 });
 
 function BasicExtensionSettingsWebUITest() {}
@@ -62,9 +167,6 @@ BasicExtensionSettingsWebUITest.prototype = {
   __proto__: ExtensionSettingsWebUITest.prototype,
 
   /** @override */
-  isAsync: true,
-
-  /** @override */
   testGenPreamble: function() {
     // Install multiple types of extensions to ensure we handle each type.
     // TODO(devlin): There are more types to add here.
@@ -76,21 +178,6 @@ BasicExtensionSettingsWebUITest.prototype = {
     GEN('  SetAutoConfirmUninstall();');
   },
 
-  /** @protected {Array<!Function>} */
-  steps: [],
-
-  /** @protected */
-  nextStep: function() {
-    assertTrue(this.steps.length > 0);
-    this.steps.shift().call(this);
-  },
-
-  /** @protected */
-  waitForPageLoad: function() {
-    var extensionList = getRequiredElement('extension-settings-list');
-    extensionList.extensionsUpdated_.then(this.nextStep.bind(this));
-  },
-
   /** @protected */
   verifyDisabledWorks: function() {
     chrome.management.setEnabled(GOOD_CRX_ID, false, function() {
@@ -119,20 +206,16 @@ BasicExtensionSettingsWebUITest.prototype = {
       });
     });
   },
-
-  /** @protected */
-  verifyDeveloperModeWorks: function() {
-    var extensionSettings = getRequiredElement('extension-settings');
-    assertFalse(extensionSettings.classList.contains('dev-mode'));
-    $('toggle-dev-on').click();
-    assertTrue(extensionSettings.classList.contains('dev-mode'));
-    chrome.developerPrivate.getProfileConfiguration(function(profileInfo) {
-      assertTrue(profileInfo.inDeveloperMode);
-      this.nextStep();
-    }.bind(this));
-  },
 };
 
+// Verify that developer mode doesn't change behavior when the number of
+// extensions changes.
+TEST_F('BasicExtensionSettingsWebUITest', 'testDeveloperModeManyExtensions',
+       function() {
+  this.testDeveloperMode();
+});
+
+
 TEST_F('BasicExtensionSettingsWebUITest', 'testDisable', function() {
   this.steps = [this.waitForPageLoad,
                 this.verifyDisabledWorks,
@@ -155,17 +238,22 @@ TEST_F('BasicExtensionSettingsWebUITest', 'testUninstall', function() {
   this.nextStep();
 });
 
-TEST_F('BasicExtensionSettingsWebUITest', 'testDeveloperMode', function() {
-  var next = this.nextStep.bind(this);
-  var checkDevModeIsOff = function() {
-    chrome.developerPrivate.getProfileConfiguration(function(profileInfo) {
-      assertFalse(profileInfo.inDeveloperMode);
-      next();
-    });
+TEST_F('BasicExtensionSettingsWebUITest', 'testNonEmptyExtensionList',
+       function() {
+  var verifyListIsNotHiddenAndEmpty = function() {
+    var listWrapper = $('extension-list-wrapper');
+    var noExtensionMessage = $('no-extensions');
+    var list = $('extension-settings-list');
+
+    assertFalse(listWrapper.hidden);
+    assertTrue(noExtensionMessage.hidden);
+    assertGT(list.childNodes.length, 0);

[Dan Beam, 2015/04/22 18:44:54]

same nits re: inline vars

[hcarmona, 2015/04/22 20:44:45]

Done.

+
+    this.nextStep();
   };
-  this.steps = [checkDevModeIsOff,
-                this.waitForPageLoad,
-                this.verifyDeveloperModeWorks,
+
+  this.steps = [this.waitForPageLoad,
+                verifyListIsNotHiddenAndEmpty,
                 testDone];
   this.nextStep();
 });
@@ -176,44 +264,17 @@ AsyncExtensionSettingsWebUITest.prototype = {
   __proto__: ExtensionSettingsWebUITest.prototype,
 
   /** @override */
-  isAsync: true,
-
-  /** @override */
   testGenPreamble: function() {
     GEN('  InstallGoodExtension();');
     GEN('  InstallErrorsExtension();');
   },
-
-  enableDeveloperMode: function(callback) {
-    var devControls = $('dev-controls');
-
-    // Make sure developer controls are hidden before checkbox is clicked.
-    assertEquals(0, devControls.offsetHeight);
-    $('toggle-dev-on').click();
-
-    document.addEventListener('webkitTransitionEnd', function f(e) {
-      if (e.target == devControls) {
-        // Make sure developer controls are not hidden after checkbox is
-        // clicked.
-        assertGT(devControls.offsetHeight, 0);
-
-        document.removeEventListener(f, 'webkitTransitionEnd');
-        callback();
-      }
-    });
-    ensureTransitionEndEvent(devControls, 4000);
-  },
 };
 
-TEST_F('AsyncExtensionSettingsWebUITest', 'testDeveloperModeA11y', function() {
-  this.enableDeveloperMode(testDone);
-});
-
 // Often times out on all platforms: http://crbug.com/467528
 TEST_F('AsyncExtensionSettingsWebUITest',
        'DISABLED_testErrorListButtonVisibility',
     function() {
-  this.enableDeveloperMode(function() {
+  var testButtonVisibility = function() {
     // 2 extensions are loaded:
     //   The 'good' extension will have 0 errors wich means no error list
     //     buttons.
@@ -231,8 +292,14 @@ TEST_F('AsyncExtensionSettingsWebUITest',
     // Hidden buttons must NOT be part of the focusRow.
     assertFalse(hiddenButtons[0].hasAttribute('column-type'));
 
-    testDone();
-  });
+    this.nextStep();
+  };
+
+  this.steps = [this.waitForPageLoad,
+                this.verifyDeveloperModeWorks,
+                testButtonVisibility,
+                testDone];
+  this.nextStep();
 });
 
 /**
@@ -240,16 +307,10 @@ TEST_F('AsyncExtensionSettingsWebUITest',
  * @extends {testing.Test}
  * @constructor
  */
-function ExtensionSettingsCommandsConfigWebUITest() {}
-
-ExtensionSettingsCommandsConfigWebUITest.prototype = {
-  __proto__: testing.Test.prototype,
+function SettingsCommandsExtensionSettingsWebUITest() {}
 
-  /** @override */
-  runAccessibilityChecks: true,
-
-  /** @override */
-  accessibilityIssuesAreErrors: true,
+SettingsCommandsExtensionSettingsWebUITest.prototype = {
+  __proto__: ExtensionSettingsWebUITest.prototype,
 
   /**
    * A URL to load before starting each test.
@@ -259,91 +320,98 @@ ExtensionSettingsCommandsConfigWebUITest.prototype = {
   browsePreload: 'chrome://extensions-frame/configureCommands',
 };
 
-TEST_F('ExtensionSettingsCommandsConfigWebUITest', 'testChromeSendHandler',
+TEST_F('SettingsCommandsExtensionSettingsWebUITest', 'testChromeSendHandler',
     function() {
   // Just navigating to the page should trigger the chrome.send().
-  assertEquals(this.browsePreload, document.location.href);
-  assertTrue($('extension-commands-overlay').classList.contains('showing'));
+  var assertOverlayVisible = function() {
+    assertTrue($('extension-commands-overlay').classList.contains('showing'));
+    this.nextStep();
+  };
+
+  this.steps = [this.waitForPageLoad,
+                assertOverlayVisible,
+                testDone];
+  this.nextStep();
 });
 
 /**
  * @constructor
  * @extends {ExtensionSettingsWebUITest}
  */
-function InstalledExtensionSettingsWebUITest() {}
+function InstallGoodExtensionSettingsWebUITest() {}
 
-InstalledExtensionSettingsWebUITest.prototype = {
+InstallGoodExtensionSettingsWebUITest.prototype = {
   __proto__: ExtensionSettingsWebUITest.prototype,
 
   /** @override */
-  typedefCppFixture: 'ExtensionSettingsUIBrowserTest',
-
-  /** @override */
   testGenPreamble: function() {
     GEN('  InstallGoodExtension();');
   },
-};
 
-/** @this {InstalledExtensionSettingsWebUITest} */
-function runAudit() {
-  assertEquals(this.browsePreload, document.location.href);
-  this.runAccessibilityAudit();
-}
-
-TEST_F('InstalledExtensionSettingsWebUITest', 'baseAccessibilityOk', runAudit);
+  emptyTestForAccessibility() {
+    this.steps = [this.waitForPageLoad,
+                  testDone];

[Dan Beam, 2015/04/22 18:44:54]

some of these fit in one line, e.g.

  this.steps = [this.waitForPageLoad, testDone];

[hcarmona, 2015/04/22 20:44:45]

Done.

+    this.nextStep();
+  },
+};
 
-/**
- * @constructor
- * @extends {InstalledExtensionSettingsWebUITest}
- */
-function AsyncInstalledExtensionSettingsWebUITest() {}
+TEST_F('InstallGoodExtensionSettingsWebUITest', 'testAccessibility',
+       function() {
+  this.emptyTestForAccessibility();
+});
 
-AsyncInstalledExtensionSettingsWebUITest.prototype = {
-  __proto__: InstalledExtensionSettingsWebUITest.prototype,
+TEST_F('InstallGoodExtensionSettingsWebUITest', 'showOptions', function() {
+  var showExtensionOptions = function() {
+    var optionsOverlay = extensions.ExtensionOptionsOverlay.getInstance();
+    optionsOverlay.setExtensionAndShowOverlay(GOOD_CRX_ID, 'GOOD!', '',
+                                              this.nextStep.bind(this));
 
-  /** @override */
-  isAsync: true,
-};
-
-TEST_F('AsyncInstalledExtensionSettingsWebUITest', 'showOptions', function() {
-  var optionsOverlay = extensions.ExtensionOptionsOverlay.getInstance();
-  optionsOverlay.setExtensionAndShowOverlay(GOOD_CRX_ID, 'GOOD!', '', testDone);
+    // Preferred size changes don't happen in browser tests. Just fake it.
+    var size = {width: 500, height: 500};
+    document.querySelector('extensionoptions').onpreferredsizechanged(size);

[Dan Beam, 2015/04/22 21:09:45]

nit: this is the same number of lines but more is more readable, IMO

+  };
 
-  // Preferred size changes don't happen in browser tests. Just fake it.
-  var size = {width: 500, height: 500};
-  document.querySelector('extensionoptions').onpreferredsizechanged(size);
+  this.steps = [this.waitForPageLoad,
+                showExtensionOptions,
+                testDone];
+  this.nextStep();
 });
 
 /**
  * @constructor
- * @extends {InstalledExtensionSettingsWebUITest}
+ * @extends {InstallGoodExtensionSettingsWebUITest}
  */
 function ManagedExtensionSettingsWebUITest() {}
 
 ManagedExtensionSettingsWebUITest.prototype = {
-  __proto__: InstalledExtensionSettingsWebUITest.prototype,
+  __proto__: InstallGoodExtensionSettingsWebUITest.prototype,
 
   /** @override */
   testGenPreamble: function() {
     GEN('  AddManagedPolicyProvider();');
-    InstalledExtensionSettingsWebUITest.prototype.testGenPreamble.call(this);
+    InstallGoodExtensionSettingsWebUITest.prototype.testGenPreamble.call(this);
   },
 };
 
-TEST_F('ManagedExtensionSettingsWebUITest', 'testAccessibility', runAudit);
+TEST_F('ManagedExtensionSettingsWebUITest', 'testAccessibility', function() {
+  this.emptyTestForAccessibility();
+});
 
 /**
  * @constructor
- * @extends {InstalledExtensionSettingsWebUITest}
+ * @extends {InstallGoodExtensionSettingsWebUITest}
  */
-function ExtensionOptionsDialogWebUITest() {}
+function OptionsDialogExtensionSettingsWebUITest() {}
 
-ExtensionOptionsDialogWebUITest.prototype = {
-  __proto__: InstalledExtensionSettingsWebUITest.prototype,
+OptionsDialogExtensionSettingsWebUITest.prototype = {
+  __proto__: InstallGoodExtensionSettingsWebUITest.prototype,
 
   /** @override */
   browsePreload: ExtensionSettingsWebUITest.prototype.browsePreload +
       '?options=' + GOOD_CRX_ID,
 };
 
-TEST_F('ExtensionOptionsDialogWebUITest', 'testAccessibility', runAudit);
+TEST_F('OptionsDialogExtensionSettingsWebUITest', 'testAccessibility',
+       function() {
+  this.emptyTestForAccessibility();
+});