739995 - Intermittent test failures in browser_dbg_bug723069_editor-breakpoints.js and others | Found the expected number of scripts. - Got 3, expected 2, followed by other errors

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P1)

Description

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

Rev3 WINNT 6.1 fx-team opt test mochitest-other on 2012-03-28 04:41:50 PDT for push 014c39d29e30


TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | Found the expected number of scripts. - Got 3, expected 2
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | breakpoint2 added, client received
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | breakpoint2 added without errors
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | Test timed out
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | correct number of editor breakpoint changes - Got 2, expected 4
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug731394_editor-contextmenu.js | Found the expected number of scripts. - Got 3, expected 2
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_script-switching.js | Found the expected number of scripts. - Got 3, expected 2
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_script-switching.js | The first script is displayed.
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_script-switching.js | The second script is displayed again.
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_script-switching.js | The first script is no longer displayed.
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_update-editor-mode.js | Found the expected number of scripts. - Got 3, expected 2
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_update-editor-mode.js | The first script is displayed.
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_update-editor-mode.js | Found the expected editor mode. - Got html, expected js

The root cause seems to be that the script list gets an extra entry for resizer.xml for some reason.

Comment 1

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

(In reply to Panos Astithas [:past] from comment #0)
> The root cause seems to be that the script list gets an extra entry for
> resizer.xml for some reason.

Apparently Firebug was also bitten by this last year:

https://groups.google.com/forum/#!topic/mozilla.dev.platform/uZpN57Qgtuc

Their fix AFAICT was to filter out chrome URLs, which I'm currently testing, too. Fixing it in the platform code would arguably be better, but probably harder, since I don't see anything related in the findScripts patch. It may require the changes mentioned in bug 384612 comment 13. CCing Jim for his opinion on this.

Comment 2

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

Attachment: Working patch

This fixes the Windows test failures locally and in a try run:

https://tbpl.mozilla.org/?tree=Try&rev=e81f0706d13a

I expect the check to be made conditional somehow during the upcoming chrome debugging support.

Comment 3

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 610551 [details] [diff] [review]
Working patch

So our list of scripts is somehow polluted. Looks like this should have a followup bug to figure out why that is.

r+ on the workaround to fix tests. (also matches my experience seeing some system scripts showing up in the drop down)

Comment 4

[Jim Blandy :jimb]

You know, I don't think the test really should be checking the number of scripts found at all. Actions which logically should have no visible effect can affect the result from findScripts. For example, if somebody happens to call the function constructor in the compartment for some innocent reason, and then entirely drop the resulting function, that function's script may or may not show up in the findScripts result, depending on whether it's been GC'd or not.

By Jason's and my thinking, this is an inescapable quirk of findScripts.

The test should, by all means, check for the presence of scripts it expects to be there. But if there are extra bits, that's not really a cause for alarm.

Comment 5

[Jim Blandy :jimb]

Talking with Jason on IRC has clarified some of this:

The resizer.xml script probably does belong in that compartment. Content can use XUL elements, and the top level of resizer.xml is made of XBL. When this happens, the elements' scripts get cloned into the content compartment. So its presence is just normal.

Arguably we shouldn't expose it to content, but as long as the script has no global set for it, it's hard to filter it out. And the debugger will never fire breakpoint hits or onEnterFrame events on uses of that script --- unless it's somehow running in the context of a debuggee global, in which case it would be fine to do so.

Comment 6

[Jason Orendorff [:jorendorff]]

Panos, I think we should do exactly what Firebug did: filter out chrome URLs when the user only wants to debug content.

Comment 7

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

(In reply to Rob Campbell [:rc] (:robcee) from comment #3)
> So our list of scripts is somehow polluted. Looks like this should have a
> followup bug to figure out why that is.

Based on Jim and Jason's comments I assume we don't need a followup bug any more, but do correct me if I'm wrong.

(In reply to Jim Blandy :jimb from comment #4)
> The test should, by all means, check for the presence of scripts it expects
> to be there. But if there are extra bits, that's not really a cause for
> alarm.

Yes, the number of scripts is probably redundant, but we use it as a guard for the following actions and checks that switch the current script based on its index in the array. I'll keep this in mind and arrange tests differently in the future.

(In reply to Jim Blandy :jimb from comment #5)
> The resizer.xml script probably does belong in that compartment. Content can
> use XUL elements, and the top level of resizer.xml is made of XBL. When this
> happens, the elements' scripts get cloned into the content compartment. So
> its presence is just normal.

What's puzzling me is why this happens only on Windows, but I guess this is something to ask XUL people.

So, in summary, I'll filter out chrome URLs as discussed and we will provide a mechanism (like Jim's suggestion of separate chrome and content actors) for the UI to choose the type of debugging it wants to perform.

Comment 8

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

https://hg.mozilla.org/integration/fx-team/rev/51e463e400d0

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/51e463e400d0

Comment 10

[Rob Campbell [:rc] (:robcee)]

(In reply to Panos Astithas [:past] from comment #7)
> (In reply to Rob Campbell [:rc] (:robcee) from comment #3)
> > So our list of scripts is somehow polluted. Looks like this should have a
> > followup bug to figure out why that is.
> 
> Based on Jim and Jason's comments I assume we don't need a followup bug any
> more, but do correct me if I'm wrong.

Nope. Sounds OK to me.