Closed Bug 1034084 Opened 7 years ago Closed 7 years ago

Reflow actor shouldn't emit reflow events after the window has been closed

Categories

(DevTools :: Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 33

People

(Reporter: pbro, Assigned: pbro)

Details

Attachments

(1 file, 1 obsolete file)

Because the actor relies on a timer to batch events together, we may hit a race condition whereby a reflow event is sent after the window has been closed.
This is particularly frequent when running tests.

Typically, at the end of an inspector mochitest-browser test, you might see this exception:

 0:07.63 console.error:
 0:07.63   Message: Error: Got an invalid root window in DocumentWalker
 0:07.63   Stack:
 0:07.64     DocumentWalker@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:2779:1
 0:07.64 documentWalker@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:2761:10
 0:07.64 WalkerActor<.parentNode@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:1116:18
 0:07.64 exports.NodeActor<.form@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:213:22
 0:07.64 types.addActorType/type<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:275:1
 0:07.64 types.addArrayType/<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:182:25
 0:07.64 Arg<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:411:12
 0:07.64 Request<.write/str<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:582:1
 0:07.64 Request<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:580:15
 0:07.64 Actor<._sendEvent@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:833:18
 0:07.64 Actor<.initialize/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:821:11
 0:07.64 emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:96:9
 0:07.64 WalkerActor<._onReflows@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:1002:7
 0:07.64 EventEmitter_emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/event-emitter.js:136:11
 0:07.64 LayoutChangesObserver.prototype<._startEventLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/layout.js:281:7
 0:07.64 notify@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/timers.js:40:9

It bubbles right up to the NodeActor because the inspector listens for reflows to determine which nodes have had their visibility change.
The solution is to test for the existence of the tabActor's window in the batching loop.
Assignee: nobody → pbrosset
This suppresses the intermittent stacktrace I've been seeing when running some of the inspector tests.
Attachment #8450195 - Flags: review?(mratcliffe)
Comment on attachment 8450195 [details] [diff] [review]
bug1034084-avoid-reflow-events-after-window-close v1.patch

Review of attachment 8450195 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, this should fix the exceptions at the end of test runs.
Attachment #8450195 - Flags: review?(mratcliffe) → review+
An xpcshell was failing with my previous patch. That's because in that test, I was mocking the TabActor and since the layout actor now relies on tabActor.attached, I needed to mock that up too.

So this new patch does just that.

New pending try: https://tbpl.mozilla.org/?tree=Try&rev=fe90cf6fd44a
Attachment #8450195 - Attachment is obsolete: true
Attachment #8450458 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3271ce25db00
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.