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

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

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.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Don't clobber the thread actor global on reload for pages that contain child frames v1

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

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

FWIW event ordering on Mac seems to be the same as Linux.

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Yep, the patch solves the Firebug issue reported here:
https://code.google.com/p/fbug/issues/detail?id=7029

Thanks Panos!
Honza

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

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.

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/2882840b841a

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2882840b841a