Last Comment Bug 706734 - "this.veilContainer is null" in browser/devtools/highlighter/inspector.jsm
: "this.veilContainer is null" in browser/devtools/highlighter/inspector.jsm
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 11
Assigned To: Paul Rouget [:paul]
:
Mentors:
Depends on:
Blocks: 702050 689920
  Show dependency treegraph
 
Reported: 2011-11-30 18:20 PST by Cameron McCormack (:heycam)
Modified: 2011-12-06 00:01 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1 (1000 bytes, patch)
2011-12-01 02:53 PST, Paul Rouget [:paul]
rcampbell: review+
Details | Diff | Review

Description Cameron McCormack (:heycam) 2011-11-30 18:20:33 PST
This exception crops up in a few inspector b-c mochitests, as a result of bug 692466 looks like.

For example see https://tbpl.mozilla.org/php/getParsedLog.php?id=7658526&tree=Try#error0 (which is a build that has uncaught-exceptions-cause-test-failure turned on):

TEST-START | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | InspectorUI variable exists
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector is not highlighting
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector.store is empty
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector is highlighting
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector Tree Panel is not open
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector Sidebar is not open
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | InspectorUI.store is not empty
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector.store.length = 1
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | selection matches the div element
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector is not highlighting
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector Tree Panel is closed
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector Sidebar is not open
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector.store.length = 1
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | an unexpected uncaught JS exception reported through window.onerror - Script error. at :0
Stack trace:
    JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: simpletestOnerror :: line 964
    native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

JavaScript error: resource:///modules/inspector.jsm, line 705: this.veilContainer is null
TEST-INFO | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Console message: [JavaScript Error: "this.veilContainer is null" {file: "resource:///modules/inspector.jsm" line: 705}]
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector is highlighting
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector Tree Panel is not open
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | Inspector.store.length = 1
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | selection matches the div element
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | sidebar is open
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | rule view is open
TEST-PASS | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | RuleView elements.length == 1
INFO TEST-END | chrome://mochitests/content/browser/browser/devtools/highlighter/test/browser_inspector_duplicate_ruleview.js | finished in 1278ms
Comment 1 Paul Rouget [:paul] 2011-12-01 02:53:40 PST
Created attachment 578215 [details] [diff] [review]
patch v1

Victor spot the problem. I didn't clear the timeout before destroying the inspector.
Comment 2 Paul Rouget [:paul] 2011-12-01 03:01:48 PST
I am not checking if `this.transitionDisabler` is null or not. This is done by `clearTimeout`.
Comment 3 Rob Campbell [:rc] (:robcee) 2011-12-01 11:35:23 PST
Comment on attachment 578215 [details] [diff] [review]
patch v1

funny, I've been hitting this testing an unrelated patch. Thanks for fixing!
Comment 4 Rob Campbell [:rc] (:robcee) 2011-12-01 11:41:11 PST
Comment on attachment 578215 [details] [diff] [review]
patch v1

actually let's move that inside closeInspectorUI() in the "if (highlighter)" section right before the destroy call.
Comment 5 Rob Campbell [:rc] (:robcee) 2011-12-01 12:11:59 PST
disregard comment #4, your patch is fine as it is.
Comment 7 Rob Campbell [:rc] (:robcee) 2011-12-02 06:04:00 PST
(In reply to Cameron McCormack (:heycam) from comment #6)
> As part of the patch, can you please remove these calls to
> ignoreAllUncaughtExceptions(), which I just added in bug 703176?
> 
> http://hg.mozilla.org/mozilla-central/file/tip/browser/devtools/highlighter/
> test/browser_inspector_duplicate_ruleview.js#l114
> 
> http://hg.mozilla.org/mozilla-central/file/tip/browser/devtools/highlighter/
> test/browser_inspector_initialization.js#l221
> 
> http://hg.mozilla.org/mozilla-central/file/tip/browser/devtools/highlighter/
> test/browser_inspector_ruleviewstore.js#l140

or you could open a separate bug to do this.
Comment 8 Paul Rouget [:paul] 2011-12-02 06:08:48 PST
(In reply to Cameron McCormack (:heycam) from comment #6)
> As part of the patch, can you please remove these calls to
> ignoreAllUncaughtExceptions(), which I just added in bug 703176?
> 
> http://hg.mozilla.org/mozilla-central/file/tip/browser/devtools/highlighter/
> test/browser_inspector_duplicate_ruleview.js#l114
> 
> http://hg.mozilla.org/mozilla-central/file/tip/browser/devtools/highlighter/
> test/browser_inspector_initialization.js#l221
> 
> http://hg.mozilla.org/mozilla-central/file/tip/browser/devtools/highlighter/
> test/browser_inspector_ruleviewstore.js#l140

Patch backed out and re-introduced in inbound. We need to land that in fx-team.

I want to try to commit this current patch myself (just got commit access level 3), so if you don't mind, I will keep it simple and just push this little change.
Comment 9 Paul Rouget [:paul] 2011-12-02 08:03:42 PST
https://hg.mozilla.org/integration/fx-team/rev/917ffaaafcd7
Comment 10 Cameron McCormack (:heycam) 2011-12-03 01:02:43 PST
(In reply to Paul Rouget [:paul] from comment #8)
> I want to try to commit this current patch myself (just got commit access
> level 3), so if you don't mind, I will keep it simple and just push this
> little change.

Yes, that's fine, I'll make those other changes myself once your patch gets merged into m-c.  Thanks!
Comment 11 Tim Taubert [:ttaubert] 2011-12-06 00:01:01 PST
https://hg.mozilla.org/mozilla-central/rev/917ffaaafcd7

Note You need to log in before you can comment on or make changes to this bug.