975372 - Australis undoReset state broken in automated test (which still passes) | gSavedState is null

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[:Gijs (he/him)]

P2 because I'm pretty sure this is showing real brokenness, even if the test passes. Example log at https://tbpl.mozilla.org/php/getParsedLog.php?id=35033501&tree=Fx-Team&full=1 . Haven't checked how long this has been happening.


02:12:33     INFO -  TEST-INFO | checking window state
02:12:33     INFO -  TEST-START | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js
02:12:33     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Entering test
02:12:33     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Not in default state to begin with
02:12:33     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Home button is in palette
02:12:33     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | The undo button is hidden before reset
02:12:33     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | In default state after reset
02:12:33     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | The undo button is visible after reset
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Not in default state after reset-undo
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | The undo button is hidden after clicking on the undo button
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Home button is in palette
02:12:34     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Leaving test
02:12:34     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Entering test
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Not in default state to begin with
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Home button is in palette
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | The undo button is hidden before reset
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | In default state after reset
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | The undo button is visible after reset
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | The undo button is hidden after another change
02:12:34     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Leaving test
02:12:34     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Entering test
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | The undo button is hidden before a reset
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | The browser should not be in default state
02:12:34     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | The undo button is visible after a reset
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | The undo reset button should be hidden after entering customization mode
02:12:35     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Leaving test
02:12:35     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Entering test
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Should be in default state at start of test
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Restore defaults button should be disabled when in default state
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Title bar button should reflect pref value
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Undo reset button should be hidden at start of test
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Restore defaults button should be enabled when pref changed
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Title bar button should reflect changed pref value
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | With titlebar flipped, no longer default
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Undo reset button should be hidden after pref change
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Restore defaults button should be disabled after reset
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Title bar button should reflect default value after reset
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Reset should reset drawInTitlebar
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | In default state after titlebar reset
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Undo reset button should be visible after reset
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Undo reset button should be enabled after reset
02:12:35     INFO -  *************************
02:12:35     INFO -  A coding exception was thrown and uncaught in a Task.
02:12:35     INFO -  Full message: TypeError: gSavedState is null
02:12:35     INFO -  Full stack: CustomizableUIInternal.undoReset@resource://app/modules/CustomizableUI.jsm:2142:1
02:12:35     INFO -  this.CustomizableUI.undoReset@resource://app/modules/CustomizableUI.jsm:2917:5
02:12:35     INFO -  CustomizeMode.prototype.undoReset/<@resource://app/modules/CustomizeMode.jsm:932:7
02:12:35     INFO -  TaskImpl_run@resource://gre/modules/Task.jsm:233:1
02:12:35     INFO -  resolve@resource://gre/modules/commonjs/sdk/core/promise.js:118:11
02:12:35     INFO -  then@resource://gre/modules/commonjs/sdk/core/promise.js:43:43
02:12:35     INFO -  resolve@resource://gre/modules/commonjs/sdk/core/promise.js:185:11
02:12:35     INFO -  resolve@resource://gre/modules/commonjs/sdk/core/promise.js:118:11
02:12:35     INFO -  then@resource://gre/modules/commonjs/sdk/core/promise.js:43:43
02:12:35     INFO -  resolve@resource://gre/modules/commonjs/sdk/core/promise.js:185:11
02:12:35     INFO -  TaskImpl_run@resource://gre/modules/Task.jsm:242:9
02:12:35     INFO -  Handler.prototype.process@resource://gre/modules/Promise.jsm:774:11
02:12:35     INFO -  this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm:653:7
02:12:35     INFO -  *************************
02:12:35     INFO -  console.error:
02:12:35     INFO -    [CustomizeMode]
02:12:35     INFO -    Message: TypeError: gSavedState is null
02:12:35     INFO -    Stack:
02:12:35     INFO -      CustomizableUIInternal.undoReset@resource://app/modules/CustomizableUI.jsm:2142:1
02:12:35     INFO -  this.CustomizableUI.undoReset@resource://app/modules/CustomizableUI.jsm:2917:5
02:12:35     INFO -  CustomizeMode.prototype.undoReset/<@resource://app/modules/CustomizeMode.jsm:932:7
02:12:35     INFO -  TaskImpl_run@resource://gre/modules/Task.jsm:233:1
02:12:35     INFO -  resolve@resource://gre/modules/commonjs/sdk/core/promise.js:118:11
02:12:35     INFO -  then@resource://gre/modules/commonjs/sdk/core/promise.js:43:43
02:12:35     INFO -  resolve@resource://gre/modules/commonjs/sdk/core/promise.js:185:11
02:12:35     INFO -  resolve@resource://gre/modules/commonjs/sdk/core/promise.js:118:11
02:12:35     INFO -  then@resource://gre/modules/commonjs/sdk/core/promise.js:43:43
02:12:35     INFO -  resolve@resource://gre/modules/commonjs/sdk/core/promise.js:185:11
02:12:35     INFO -  TaskImpl_run@resource://gre/modules/Task.jsm:242:9
02:12:35     INFO -  Handler.prototype.process@resource://gre/modules/Promise.jsm:774:11
02:12:35     INFO -  this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm:653:7
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Restore defaults button should be enabled after undo-reset
02:12:35     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Title bar button should reflect undo-reset value
02:12:36     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | No longer in default state after undo
02:12:36     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Undo-reset goes back to previous pref value
02:12:36     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Undo reset button should be hidden after undo-reset clicked
02:12:36     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | In default state after pref cleared
02:12:36     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Undo reset button should be hidden at end of test
02:12:36     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Leaving test
02:12:36     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Entering test asyncCleanup
02:12:36     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_970511_undo_restore_default.js | Leaving test asyncCleanup

Comment 1

[:Gijs (he/him)]

Attachment: make Australis' customizeMode's undoReset not error out at the end when the gSavedState is null,

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

On a side note, why doesn't the following fail the test:
> A coding exception was thrown and uncaught in a Task.
> Full message: TypeError: gSavedState is null

Things like this shouldn't go unnoticed and should trigger an orange.

Comment 3

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/c1e26b826011

Comment 4

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)
> On a side note, why doesn't the following fail the test:
> > A coding exception was thrown and uncaught in a Task.
> > Full message: TypeError: gSavedState is null
> 
> Things like this shouldn't go unnoticed and should trigger an orange.

Agreed. Filed bug 976205.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c1e26b826011

Comment 6

[:Gijs (he/him)]

Comment on attachment 8379691 [details] [diff] [review]
make Australis' customizeMode's undoReset not error out at the end when the gSavedState is null,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis / bug which added this test (which I can't find right now)
User impact if declined: users might see spurious error messages in the console; developers see spurious error messages on automated tests
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): none, simple nullcheck
String or IDL/UUID changes made by this patch: none

Comment 7

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/904794b089c2