Closed Bug 1574190 Opened 5 years ago Closed 5 years ago

Add server support for setting watchpoints

Categories

(DevTools :: Debugger, enhancement, P1)

enhancement

Tracking

(firefox70+ fixed, firefox71 fixed)

RESOLVED FIXED
Firefox 70
Tracking Status
firefox70 + fixed
firefox71 --- fixed

People

(Reporter: jlast, Assigned: bmiriam1230)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

The server should have endpoint for adding and removing (get|set) watchpoints

No longer blocks: dbg-watchpoints
Depends on: 1574192
No longer depends on: 1574192
Priority: -- → P3

Add server support for watchpoints

Attachment #9087102 - Attachment is obsolete: true
Attachment #9087102 - Attachment is obsolete: false

Lint and add test.

Add server support for watchpoints.

Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e22b66d70f9
Add server support for watchpoints. r=jlast
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
Assignee: nobody → bmiriam1230
Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b0a5b50962e7
Add watchpoint as a trait in the server, remove watchpoints when object actor is released, add watchpoint to property descriptor object r=jlast
Pushed by jlaster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cc3c6d62c9e6
Add watchpoints trait to browsingContextTargetPrototype and remove from RootActor r=jlast
Attached file commits to uplift

Beta/Release Uplift Approval Request

  • User impact if declined: the vscode debugger will not have support for watchpoints for another cycle
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): the server code is not executed unless a watchpoint is added
  • String changes made/needed:
Attachment #9092119 - Flags: approval-mozilla-beta?
Attachment #9087155 - Flags: approval-mozilla-beta?
Attachment #9091090 - Flags: approval-mozilla-beta?

Do we have a reason to rush (part of) this to release? Conversely would letting it stay in nightly for a while help uncover potential bugs? Is there any testing/QA we can do around this feature?

Flags: needinfo?(jlaster)
Flags: needinfo?(bmiriam1230)

Also noticing that some of this landed in 70 nightly (so it's already marked as "fixed" for 70, which is likely not correct)

We hope that this feature will be helpful for users who would like to debug firefox 70 from vs code.

Flags: needinfo?(jlaster)

Do we have a reason to rush (part of) this to release?

VSCode's Firefox debugger is an extension developed off the train for VSCode, to which we added Watchpoint support. Given that some of the patches landed in 70, it seemed low-risk to uplift the remaining work.

Conversely would letting it stay in nightly for a while help uncover potential bugs?

Maybe, as we just landed UI for it in DevTools.

Is there any testing/QA we can do around this feature?

We planned to let Watchpoints be Nightly to collect feedback from internal users.

Flags: needinfo?(bmiriam1230)

OK, I just found the planning info in the browser engineering board.
Why don't we go ahead and uplift for beta 7 (right at the end of "early beta") since this missed today's build.

Attachment #9087155 - Flags: approval-mozilla-beta?

Thanks Liz, that sounds good! Is there anything else we need to update in this bug?

Flags: needinfo?(lhenry)
Flags: needinfo?(lhenry)

Changing the priority to p1 as the bug is tracked by a release manager for the current beta.
See What Do You Triage for more information

Priority: P3 → P1
Comment on attachment 9092119 [details]
commits to uplift

Baked 2 weeks in Nightly without reported regressions, covered by tests  and we are now past the "Early Beta" phase, uplift approved for 79 beta 9, thanks.
Attachment #9092119 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9091090 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9091090 - Flags: approval-mozilla-beta+

Note to sheriffs for the beta uplift. The accepted uplift patch contains the message with the 2 revisions to uplift:

It'd be great to uplift these two commits: https://hg.mozilla.org/mozilla-central/rev/8e22b66d70f9 https://hg.mozilla.org/mozilla-central/rev/b0a5b50962e7

Jason, I'm getting the error "note: graft of 562003:8e22b66d70f9 created no changes to commit" when using graft to uplift the patch to beta and conflicts when I tried importing the patches with moz-phab.

Flags: needinfo?(jlaster)

The first revision landed for Gecko 70 and didn't need uplift.

Flags: needinfo?(jlaster)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: