TEST-UNEXPECTED-FAIL | page-mod-debugger-post/main.testDebugger || TypeError: debuggee.runDebuggerStatement is not a function

RESOLVED FIXED

Status

defect
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: zombie, Assigned: zer0)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

nothing that landed recently in jetpack looks like it touched debugger stuff, so this is probably a devtools issue (from bug 897567 maybe?), though Wes informs me the fx-team tree is green atm..

https://tbpl.mozilla.org/php/getParsedLog.php?id=44459560&tree=Jetpack
Summary: TEST-UNEXPECTED-FAIL | page-mod-debugger-post/main.testDebugger | Test timed out → TEST-UNEXPECTED-FAIL | page-mod-debugger-post/main.testDebugger || TypeError: debuggee.runDebuggerStatement is not a function
Erik, you wrote this test. Any ideas what might have broken it?
Flags: needinfo?(evold)
Priority: -- → P1
note: although it looks like this started failing between two of my pushes yesterday, it's totally unrelated. the cause is somewhere in the yesterday's Nightly (i re-triggered a few test runs after it was built, and they fail in the same way).
(In reply to Dave Townsend [:mossop] from comment #1)
> Erik, you wrote this test. Any ideas what might have broken it?

Something in the devtools code base probably changed, I may not be the best person to look in to this.

If you want me to though I will.
Flags: needinfo?(evold) → needinfo?(dtownsend+bugmail)
Assignee: nobody → zer0
The bug was caused by set directly the function in `unsafeWindow`; using `exportFunction` instead fix the issue.
Attachment #8462820 - Flags: review?(evold)
Duplicate of this bug: 1044088
Attachment #8462820 - Flags: review?(evold) → review?(jsantell)
Attachment #8462820 - Flags: review?(jsantell) → review+
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/0efe04f0526d15f45caf7775c3e9d7791c71b289
Bug 1042976 - TypeError: debuggee.runDebuggerStatement is not a function

- Used `exportFunction` in test addons

https://github.com/mozilla/addon-sdk/commit/55efdef507c2023ba87268e8ff1726f6237328af
Merge pull request #1565 from ZER0/runDebuggerStatement/1042976

fix Bug 1042976 - TypeError: debuggee.runDebuggerStatement is not a function r=@jsantell
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
This test is just timing out now
Status: RESOLVED → REOPENED
Flags: needinfo?(dtownsend+bugmail)
Resolution: FIXED → ---
(In reply to Dave Townsend [:mossop] from comment #7)
> This test is just timing out now

This started failing on this push of checkin-needed bugs to fx-team: https://tbpl.mozilla.org/?tree=Fx-Team&jobname=jetpack&showall=1&rev=fea2a90e2d22

Gonna guess it's bug 1039952.
Flags: needinfo?(nfitzgerald)
It might be related, as bug 1039952 did change the way that debuggee globals are managed by the debugger, which includes the addon debugger. I'm not sure what these tests are, or where exactly the test is located. Those files linked don't tell me much...

Mossop, you wrote a lot of the initial addon debugger code surrounding finding addon debuggee globals, can you take a look at [0] and see if there is anything missing from the original logic?

[0] http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/webbrowser.js#1492
Flags: needinfo?(nfitzgerald)
This test isn't part of the add-on debugger, it's page-mods showing up in the regular webpage debugger I think, that was Erik's work. Erik, how does the page-mod globals get added as debuggees?
Flags: needinfo?(evold)
Is it was correct reopen this bug? The previous error was fixed, now is going in timeout for a totally different reason – on the same test, true, but `debuggee.runDebuggerStatement` now is a function.
Blocks: 1049215
(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #12)
> Is it was correct reopen this bug? The previous error was fixed, now is
> going in timeout for a totally different reason – on the same test, true,
> but `debuggee.runDebuggerStatement` now is a function.

Split this new stuff off into bug 1049215. Re-resolving this bug.
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
(In reply to Nick Fitzgerald [:fitzgen] from comment #11)
> In that case, for finding all debuggee globals, we use the parent's
> |windows| getter:
> 
> http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> webbrowser.js#551
> http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> webbrowser.js#635
> 
> And for new globals that pop up while debugging, we check if we should add
> them as a debuggee with this function:
> 
> http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> webbrowser.js#754

So the change to `onNewGlobal` in https://hg.mozilla.org/mozilla-central/diff/fea2a90e2d22/toolkit/devtools/server/actors/script.js is what caused this test to break.

Basically the new `onNewGlobal` is ignoring page-mods now it seems to me.

Any ideas how we can resolve this Nick?

Can we keep the bit of code that was added in https://github.com/mozilla/gecko-dev/commit/34c7599e4ea29439e8ba6b3f95bd2b3ab72f9f2d#diff-58d9dfd9ca670d23e714c743a045d90eR661 ?
Flags: needinfo?(evold) → needinfo?(nfitzgerald)
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #14)
> (In reply to Nick Fitzgerald [:fitzgen] from comment #11)
> > In that case, for finding all debuggee globals, we use the parent's
> > |windows| getter:
> > 
> > http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> > webbrowser.js#551
> > http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> > webbrowser.js#635
> > 
> > And for new globals that pop up while debugging, we check if we should add
> > them as a debuggee with this function:
> > 
> > http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> > webbrowser.js#754
> 
> So the change to `onNewGlobal` in
> https://hg.mozilla.org/mozilla-central/diff/fea2a90e2d22/toolkit/devtools/
> server/actors/script.js is what caused this test to break.
> 
> Basically the new `onNewGlobal` is ignoring page-mods now it seems to me.
> 
> Any ideas how we can resolve this Nick?
> 
> Can we keep the bit of code that was added in
> https://github.com/mozilla/gecko-dev/commit/
> 34c7599e4ea29439e8ba6b3f95bd2b3ab72f9f2d#diff-
> 58d9dfd9ca670d23e714c743a045d90eR661 ?

We still have that code, but its in the tab actor now: http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/webbrowser.js#754

Debuggees are now managed by the parent actor (tab/root/addon), not the thread actor directly.
Flags: needinfo?(nfitzgerald)
disabling tests temporarily, a=Mossop
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/c2606b723f2d895efb741403053d5c87c7fdcb05
bug 1042976 - temporarily disable pagemod debugger tests

https://github.com/mozilla/addon-sdk/commit/7db4aade3d69a2ee87c9ee2c001812aca32e1759
Merge pull request #1574 from zombie/1042976-debugger-tests

bug 1042976 - temporarily disable pagemod debugger tests, a=@Mossop
(In reply to Nick Fitzgerald [:fitzgen] from comment #15)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #14)
> > (In reply to Nick Fitzgerald [:fitzgen] from comment #11)
> > > In that case, for finding all debuggee globals, we use the parent's
> > > |windows| getter:
> > > 
> > > http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> > > webbrowser.js#551
> > > http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> > > webbrowser.js#635
> > > 
> > > And for new globals that pop up while debugging, we check if we should add
> > > them as a debuggee with this function:
> > > 
> > > http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> > > webbrowser.js#754
> > 
> > So the change to `onNewGlobal` in
> > https://hg.mozilla.org/mozilla-central/diff/fea2a90e2d22/toolkit/devtools/
> > server/actors/script.js is what caused this test to break.
> > 
> > Basically the new `onNewGlobal` is ignoring page-mods now it seems to me.
> > 
> > Any ideas how we can resolve this Nick?
> > 
> > Can we keep the bit of code that was added in
> > https://github.com/mozilla/gecko-dev/commit/
> > 34c7599e4ea29439e8ba6b3f95bd2b3ab72f9f2d#diff-
> > 58d9dfd9ca670d23e714c743a045d90eR661 ?
> 
> We still have that code, but its in the tab actor now:
> http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> webbrowser.js#754
> 
> Debuggees are now managed by the parent actor (tab/root/addon), not the
> thread actor directly.

Ah so there is a `getInnerId in undefined` error that is being silenced.
Flags: needinfo?(nfitzgerald)
But it is the same that it used to be: http://hg.mozilla.org/mozilla-central/rev/fea2a90e2d22#l5.288
Flags: needinfo?(nfitzgerald)
(In reply to Nick Fitzgerald [:fitzgen] from comment #19)
> But it is the same that it used to be:
> http://hg.mozilla.org/mozilla-central/rev/fea2a90e2d22#l5.288

So the call to `getInnerId(...)` got moved from a file that had a getInnerId function defined to a file that does not have it defined.

From http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/script.js
To http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/webbrowser.js

So the `getInnerId` function in the former file should be moved to the latter file I suppose.
Flags: needinfo?(nfitzgerald)
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #20)
> (In reply to Nick Fitzgerald [:fitzgen] from comment #19)
> > But it is the same that it used to be:
> > http://hg.mozilla.org/mozilla-central/rev/fea2a90e2d22#l5.288
> 
> So the call to `getInnerId(...)` got moved from a file that had a getInnerId
> function defined to a file that does not have it defined.
> 
> From
> http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> script.js
> To
> http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> webbrowser.js
> 
> So the `getInnerId` function in the former file should be moved to the
> latter file I suppose.

D'oh! Good catch. Will have a patch up shortly.
Flags: needinfo?(nfitzgerald)
(In reply to Nick Fitzgerald [:fitzgen] from comment #21)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #20)
> > (In reply to Nick Fitzgerald [:fitzgen] from comment #19)
> > > But it is the same that it used to be:
> > > http://hg.mozilla.org/mozilla-central/rev/fea2a90e2d22#l5.288
> > 
> > So the call to `getInnerId(...)` got moved from a file that had a getInnerId
> > function defined to a file that does not have it defined.
> > 
> > From
> > http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> > script.js
> > To
> > http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> > webbrowser.js
> > 
> > So the `getInnerId` function in the former file should be moved to the
> > latter file I suppose.
> 
> D'oh! Good catch. Will have a patch up shortly.

Did that ever happen?
Flags: needinfo?(nfitzgerald)
(In reply to Dave Townsend [:mossop] from comment #22)
> (In reply to Nick Fitzgerald [:fitzgen] from comment #21)
> > (In reply to Erik Vold [:erikvold] [:ztatic] from comment #20)
> > > (In reply to Nick Fitzgerald [:fitzgen] from comment #19)
> > > > But it is the same that it used to be:
> > > > http://hg.mozilla.org/mozilla-central/rev/fea2a90e2d22#l5.288
> > > 
> > > So the call to `getInnerId(...)` got moved from a file that had a getInnerId
> > > function defined to a file that does not have it defined.
> > > 
> > > From
> > > http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> > > script.js
> > > To
> > > http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/
> > > webbrowser.js
> > > 
> > > So the `getInnerId` function in the former file should be moved to the
> > > latter file I suppose.
> > 
> > D'oh! Good catch. Will have a patch up shortly.
> 
> Did that ever happen?

Yeah, but looks like we have a leftover copy of the function in the old file, hence the followup patch.
Flags: needinfo?(nfitzgerald)
Attachment #8577402 - Flags: review?(dtownsend) → review+
Well the test is still failing for some reason, filed bug 1143136
You need to log in before you can comment on or make changes to this bug.