Closed Bug 962632 Opened 6 years ago Closed 6 years ago

Reloading a page that contains child frames may clobber the thread actor global

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 29

People

(Reporter: past, Assigned: past)

Details

Attachments

(1 file)

BTA_onWindowCreated resets the thread actor global on page reload or navigation. Due to an unfortunate oversight, the part that does this gets executed for pageshow events too, besides the intended DOMWindowCreated events. Apart from grossly inefficient, Honza tells me that on Windows this can result in a child window replacing the thread actor global, which precludes debugging of the main page.

In my testing on Linux this doesn't happen due to a fortunate sequence of events:

- the first DOMWindowCreated event comes from the contentWindow and sets the global
- then any following DOMWindowCreated events come from the iframes and reset the global
- next we get pageshow events for all windows that reset the global again, but luckily they are triggered from the innermost window to the outermost, so in the end threadActor.global is set to the right value

If Windows shows different behavior it probably means that this order is not defined in the spec and at any rate we shouldn't be relying on it.
Changed the code so that the thread actor global is reset only for DOMWindowCreated events for the main window.

Try: https://tbpl.mozilla.org/?tree=Try&rev=f1c36cc6c5e8
Assignee: nobody → past
Status: NEW → ASSIGNED
FWIW event ordering on Mac seems to be the same as Linux.
Yep, the patch solves the Firebug issue reported here:
https://code.google.com/p/fbug/issues/detail?id=7029

Thanks Panos!
Honza
Priority: -- → P2
Comment on attachment 8363733 [details] [diff] [review]
Don't clobber the thread actor global on reload for pages that contain child frames v1

I can't think of a way to write a test that will pass with this patch, but fail without it (at least on Mac and Linux), so I'm inclined to not bother. It has survived all of my tests and a try run, so I think I'll just go ahead and ask for a review as it is.
Attachment #8363733 - Flags: review?(nfitzgerald)
Attachment #8363733 - Flags: review?(nfitzgerald) → review+
https://hg.mozilla.org/mozilla-central/rev/2882840b841a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.