Closed Bug 1543907 Opened 6 months ago Closed 6 months ago

devtools.showToolbox can return too early if called while tool is selected

Categories

(DevTools :: General, defect, P3)

defect

Tracking

(firefox68 fixed)

RESOLVED FIXED
Firefox 68
Tracking Status
firefox68 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(1 file)

This is a follow up to Bug 1518005. See https://phabricator.services.mozilla.com/D27160#798175 for the context.

STRs (probably impossible to trigger manually, use this as a basis to write a test)

  • open toolbox
  • select a tool that is slow to open
  • before the tool has finished to load, call await devtools.showToolbox() for the same toolId as in step2
  • call toolbox.getPanel() for the same toolId

ER: getPanel should return the panel
AR: getPanel returns null

The reason for that is that showToolbox() returns too early if we are already in the middle of switching to toolId, thanks to https://searchfox.org/mozilla-central/rev/6dab6dad9cc852011a14275a8b2c2c03ed7600a7/devtools/client/framework/devtools.js#460

showToolbox(toolId) should wait for getPanelWhenReady(toolId), even if the toolId is the same as the current one. We should have a mochitest too.

(In reply to Brendan Dahl [:bdahl] from Bug 1518005 comment #14)

Btw, there looks to be a lot of code that uses a similar method and there appears to be some other intermittents similar to this.

Do you have a list of other intermittents that looked similar? I doubt we have the same pattern as in the other test that frequently. And await showToolbox should be fine in a "regular" scenario. But if there are other similar looking intermittents I could take a look at the same time.

Flags: needinfo?(bdahl)
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: -- → P3

(In reply to Julian Descottes [:jdescottes] from comment #1)

(In reply to Brendan Dahl [:bdahl] from Bug 1518005 comment #14)

Btw, there looks to be a lot of code that uses a similar method and there appears to be some other intermittents similar to this.

Do you have a list of other intermittents that looked similar? I doubt we have the same pattern as in the other test that frequently. And await showToolbox should be fine in a "regular" scenario. But if there are other similar looking intermittents I could take a look at the same time.

I had done a quick search for "createDebuggerContext" and there were a number of uses. I also looked for panel is undefined bugs and there seem to be a few, though looking through them I'm not sure they are related.

Flags: needinfo?(bdahl)
Blocks: 1544081

I assume you meant to r+ the patch on phabricator ? :)

Flags: needinfo?(bgrinstead)
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/247f4004f264
showToolbox should wait for tool to be loaded;r=bgrins
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
Flags: needinfo?(bgrinstead)
You need to log in before you can comment on or make changes to this bug.