1502224 - Update Debugger Frontend v96

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement, P3)

Description

[David Walsh :davidwalsh]

https://github.com/devtools-html/debugger.html/compare/release-95...release-96

Comment 1

[David Walsh :davidwalsh]

Attachment: 1502224-01.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a7e873d1bbf80f1d3d9fe71080b20fa04599249

Comment 2

[David Walsh :davidwalsh]

Attachment: 1502224-02.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f5a85523066aa486694a5adf6d6a8d6aa2c2c4a

Comment 3

[Julian Descottes [:jdescottes]]

Comment on attachment 9020254 [details] [diff] [review]
1502224-01.patch

I suppose the first patch is obsolete?

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 9020258 [details] [diff] [review]
1502224-02.patch

Review of attachment 9020258 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks! A few comments that we need to look at before landing.

::: devtools/client/debugger/new/src/moz.build
@@ +13,4 @@
>      'workers',
>  ]
>  
> +DevToolsModules(

Do we no longer need the DebuggerModules instruction? Should we delete https://searchfox.org/mozilla-central/source/devtools/client/debugger/new/node-templates.mozbuild ? Haven't followed the topic closely lately.

::: devtools/client/debugger/new/test/mochitest/browser_dbg-scopes.js
@@ +8,4 @@
>  add_task(async function() {
>    const dbg = await initDebugger("doc-script-switching.html");
>  
> +  const ready = Promise.all([

the patch doesn't apply because this change is already on central from https://hg.mozilla.org/mozilla-central/rev/0f30ba8cfeec.

Needs to be rebased.

::: devtools/client/debugger/new/test/mochitest/browser_dbg-sources-querystring.js
@@ +15,5 @@
> +    selectors: { getSelectedSource },
> +    getState
> +  } = dbg;
> +
> +  await waitForSources(dbg, "simple1.js?x=1", "simple1.js?x=2");

The test often blocks here. It seems to be super frequent on OSX debug.
I can't reproduce the issue locally for now, even thought I am using debug builds on OSX with --verify.

Maybe skip on this platform if you can't figure out why this is failing? With a bugged (or GH issue) logged to investigate it.

Comment 5

[David Walsh :davidwalsh]

Attachment: 1502224-03.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f441a0ad1d2fda72a6d495490c1471468ad6c4c

Comment 6

[David Walsh :davidwalsh]

Attachment: Update debugger frontend v96 r=jdescottes

Comment 7

[David Walsh :davidwalsh]

Attachment: Bug 1502224 - Update debugger frontend v96 r=jdescottes

Comment 8

[David Walsh :davidwalsh]

Attachment: 1502224-04.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=59f88434009d15d88731f658a1f0b177dbf2d008

Comment 9

[Julian Descottes [:jdescottes]]

Some questions about the last update:
- which patch should I review, the one on splinter or the one on phabricator? 
- looking at try the querystring test still seems very intermittent, not sure if anything changed about it?
- regarding my question about "DebuggerModules" vs "DevToolsModules", I see the change is gone now, but I would still like to know if we plan to remove it since it's only used in one spot?

Comment 10

[David Walsh :davidwalsh]

- They were both the same, but I closed the Phabricator revision to avoid confusion.
- Weird; we've had 0 problems with it on travis.  I'll look into it.
- I'll talk to Alex about "DebuggerModules" vs "DevToolsModules" -- I don't know what it does and didn't want to change it.

Comment 11

[Julian Descottes [:jdescottes]]

Comment on attachment 9020434 [details] [diff] [review]
1502224-04.patch

Review of attachment 9020434 [details] [diff] [review]:
-----------------------------------------------------------------

The patch is fine codewise, but given the intermittent failures on try this might be backed out.

As mentioned in my first review, can we:
- skip the test
- file a follow up bug or GH issue to reenable it
- skip-if=true should be followed by a comment linking to the bug/issue

Comment 12

[David Walsh :davidwalsh]

Attachment: 1502224-05.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=79f7abd11cd713cb7af1ace331d59b79e426f468

Skipped querystring test, will debug separately.

Comment 13

[Tim Nguyen :ntim]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc381d0178d261adb8b8a5146d2db0ae85a98e3f

Comment 14

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/bc381d0178d2