Closed Bug 1535246 Opened 5 years ago Closed 5 years ago

Breakpoint positions are only fetched for some source actors in HTML files

Tracking

(firefox68 affected)

Status:

RESOLVED INVALID

Milestone:

Firefox 68

Tracking Flags:

Tracking

Status

firefox68

---

affected

People

(Reporter: loganfsmyth, Assigned: loganfsmyth)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files)

Bug 1535246 - Part 1: Ensure that changes to emptyLines are handled for both additions and removals. r=davidwalsh 5 years ago Logan Smyth [:loganfsmyth] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1535246 - Part 2: Handle new actors appearing in HTML files. r=davidwalsh 5 years ago Logan Smyth [:loganfsmyth] 47 bytes, text/x-phabricator-request		Details \| Review

Logan Smyth [:loganfsmyth]

Assignee

Description

•

5 years ago

This was mentioned in a comment in https://bugzilla.mozilla.org/show_bug.cgi?id=1527297#c8 but I'm refiling it here since I don't think it is related.

Steps:

Create an example HTML file with this:

<html>
  <head>
    <script type="text/javascript">
      function test() {
        var something = "you can add";
        something += " breakpoints here";
        console.log(something);
      }
    </script>
    <script type="text/javascript">
      function failing() {
        var wrong = "you cannot add";
        wrong += " breakpoints in this block";
        console.log(wrong);
      }
    </script>
  <body>
  </body>
</html>

Open the file in Firefox Nightly

AR:
Breakpoints/empty-lines will only show up for one of the scripts. It seems to be a bit random in my tests.

ER:
All breakpoints for all actors should be fetched.

I tried changing getBreakpointPositions in commands.js to fetch the BPs for all actors, but that only seems to make things flakey. I won't have time to dig into it more. I'm thinking even with that we also need to request BPs any time a new actor is added to the source, not just when the first actor is found?

David Walsh :davidwalsh

Updated

•

5 years ago

Priority: -- → P3

Logan Smyth [:loganfsmyth]

Assignee

Comment 1

•

5 years ago

I'm surprised about this being P3. Isn't this kind of a core behavior that we've broken?

David Walsh :davidwalsh

Updated

•

5 years ago

Priority: P3 → P2

:Harald Kirschner :digitarald

Comment 2

•

5 years ago

As decided in the debugger meeting, making this a blocker.

Severity: normal → blocker

Priority: P2 → P1

Logan Smyth [:loganfsmyth]

Assignee

Comment 5

•

5 years ago

Attached file Bug 1535246 - Part 1: Ensure that changes to emptyLines are handled for both additions and removals. r=davidwalsh — Details

Logan Smyth [:loganfsmyth]

Assignee

Comment 6

•

5 years ago

Attached file Bug 1535246 - Part 2: Handle new actors appearing in HTML files. r=davidwalsh — Details

Depends on D24443

Pulsebot

Comment 7

•

5 years ago

Pushed by lsmyth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5929d34ee923
Part 1: Ensure that changes to emptyLines are handled for both additions and removals. r=davidwalsh
https://hg.mozilla.org/integration/autoland/rev/5860aac31563
Part 2: Handle new actors appearing in HTML files. r=davidwalsh

Cosmin Sabou [:CosminS]

Comment 8

•

5 years ago

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&selectedJob=235590898&revision=5860aac315635b8a0103d96938a14f03bf122f4e

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=235590898&repo=autoland&lineNumber=4112

Backout link: https://hg.mozilla.org/integration/autoland/rev/ea922e6043f867cd054d557d5c9aed5ba539357d

[task 2019-03-23T00:04:34.980Z] 00:04:34 [task 2019-03-23T00:04:34.980Z] 00:04:34 [task 2019-03-23T00:04:34.981Z] 00:04:34 [task 2019-03-23T00:04:34.981Z] 00:04:34 [task 2019-03-23T00:04:34.981Z] 00:04:34 [task 2019-03-23T00:04:34.981Z] 00:04:34 [task 2019-03-23T00:04:34.981Z] 00:04:34 [task 2019-03-23T00:04:34.981Z] 00:04:34 [task 2019-03-23T00:04:34.982Z] 00:04:34 [task 2019-03-23T00:04:34.982Z] 00:04:34 [task 2019-03-23T00:04:34.982Z] 00:04:34 [task 2019-03-23T00:04:34.982Z] 00:04:34 [task 2019-03-23T00:04:34.982Z] 00:04:34 [task 2019-03-23T00:04:34.983Z] 00:04:34 [task 2019-03-23T00:04:34.983Z] 00:04:34 [task 2019-03-23T00:04:34.984Z] 00:04:34 [task 2019-03-23T00:04:34.984Z] 00:04:34 [task 2019-03-23T00:04:34.984Z] 00:04:34 [task 2019-03-23T00:04:34.984Z] 00:04:34 [task 2019-03-23T00:04:34.985Z] 00:04:34 [task 2019-03-23T00:04:34.985Z] 00:04:34 [task 2019-03-23T00:04:34.985Z] 00:04:34 [task 2019-03-23T00:04:34.985Z] 00:04:34 [task 2019-03-23T00:04:34.986Z] 00:04:34 [task 2019-03-23T00:04:34.986Z] 00:04:34 [task 2019-03-23T00:04:35.173Z] 00:04:35 [task 2019-03-23T00:04:35.174Z] 00:04:35 [task 2019-03-23T00:04:35.230Z] 00:04:35 [task 2019-03-23T00:04:35.247Z] 00:04:35 [task 2019-03-23T00:04:35.304Z] 00:04:35 [task 2019-03-23T00:04:35.304Z] 00:04:35 INFO - TEST-PASS | devtools/client/debugger/new/test/mochitest/browser_dbg-html-breakpoints.js | a new breakpoint was created -
INFO - Waiting for state change: waiting for breakable line 20
INFO - Console message: [JavaScript Error: "Polling for changes failed: Unexpected content-type "text/html;charset=utf-8"." {file: "resource://services-settings/remote-settings.js" line: 203}]
INFO - remoteSettingsFunction/remoteSettings.pollChanges@resource://services-settings/remote-settings.js:203:13
INFO - asyncnotify@resource://services-settings/RemoteSettingsComponents.jsm:22:20
INFO - TM_notify/<@resource://gre/modules/UpdateTimerManager.jsm:192:48
INFO - TM_notify@resource://gre/modules/UpdateTimerManager.jsm:239:7
INFO -
INFO - Buffered messages logged at 00:03:48
INFO - Console message: [JavaScript Error: "getScreenshot(http://example.com/browser/devtools/client/debugger/new/test/mochitest/examples/doc-scripts.html) failed: TypeError: NetworkError when attempting to fetch resource." {file: "resource://activity-stream/lib/Screenshots.jsm" line: 45}]
INFO - getScreenshotForURL@resource://activity-stream/lib/Screenshots.jsm:45:10
INFO - asyncmaybeCacheScreenshot@resource://activity-stream/lib/Screenshots.jsm:98:37
INFO - _fetchScreenshot@resource://activity-stream/lib/TopSitesFeed.jsm:437:23
INFO - _fetchIcon@resource://activity-stream/lib/TopSitesFeed.jsm:425:16
INFO - getLinksWithDefaults@resource://activity-stream/lib/TopSitesFeed.jsm:328:16
INFO - async*refresh@resource://activity-stream/lib/TopSitesFeed.jsm:351:30
INFO - onAction@resource://activity-stream/lib/TopSitesFeed.jsm:650:14
INFO - _middleware/</<@resource://activity-stream/lib/Store.jsm:51:17
INFO - Store/this[method]@resource://activity-stream/lib/Store.jsm:29:54
INFO - init/this.intervalId<@resource://activity-stream/lib/SystemTickFeed.jsm:16:52
INFO - notify@resource://gre/modules/Timer.jsm:43:17
INFO -
INFO - Buffered messages finished
INFO - TEST-UNEXPECTED-FAIL | devtools/client/debugger/new/test/mochitest/browser_dbg-html-breakpoints.js | Test timed out -
INFO - GECKO(1411) | console.warn: "Error while detaching the thread front: 'detach' request packet to 'server1.conn36.child1/context18' can't be sent as the connection is closed."
INFO - Removing tab.
INFO - Waiting for event: 'TabClose' on [object XULElement].
INFO - Got event: 'TabClose' on [object XULElement].
INFO - Tab removed and finished closing
INFO - GECKO(1411) | MEMORY STAT | vsize 20975023MB | residentFast 2074MB
INFO - TEST-OK | devtools/client/debugger/new/test/mochitest/browser_dbg-html-breakpoints.js | took 90806ms

Flags: needinfo?(lsmyth)

Pulsebot

Comment 9

•

5 years ago

Pushed by lsmyth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6fe3959c6558
Part 1: Ensure that changes to emptyLines are handled for both additions and removals. r=davidwalsh
https://hg.mozilla.org/integration/autoland/rev/fd22f8527dfb
Part 2: Handle new actors appearing in HTML files. r=davidwalsh

Logan Smyth [:loganfsmyth]

Assignee

Updated

•

5 years ago

Assignee: nobody → lsmyth

Status: NEW → ASSIGNED

Flags: needinfo?(lsmyth)

Andrei Ciure[:aciure]

Comment 10

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/6fe3959c6558
https://hg.mozilla.org/mozilla-central/rev/fd22f8527dfb

Status: ASSIGNED → RESOLVED

Closed: 5 years ago

status-firefox68: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 68

Jason Laster [:jlast]

Comment 11

•

5 years ago

I just tried reverting the empty lines patch to see if it will speed up the stepIn example.

To hg::ssh://hg.mozilla.org/try

6bed605c756d...d2f028b9dd9b HEAD -> branches/default/tip (forced update)
remote:
remote: View your changes here:
remote: https://hg.mozilla.org/try/rev/60e79172c3c46b1a64418df62ea879cf11b0e96c
remote: https://hg.mozilla.org/try/rev/aca2b18af5f80d6a94cf13bdf5bb957bc7db00e1
remote: https://hg.mozilla.org/try/rev/2271c176332723ba1709ce4490fb471afc4c032a
remote:
remote: Follow the progress of your build on Treeherder:
remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2271c176332723ba1709ce4490fb471afc4c032a
remote:
remote: It looks like this try push has talos jobs. Compare performance against a baseline revision:
remote: https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=2271c176332723ba1709ce4490fb471afc4c032a
remote: recorded changegroup in replication log in 0.015s

Jason Laster [:jlast]

Comment 12

•

5 years ago

re-opening so we can get an answer on the performance regression.

Step-in regressed by 200% and step-over by 38% on these changes

Status: RESOLVED → REOPENED

Resolution: FIXED → ---

BugBot [:suhaib / :marco/ :calixte]

Updated

•

5 years ago

status-firefox68: fixed → affected

:Harald Kirschner :digitarald

Updated

•

5 years ago

Blocks: dbg-perf

Keywords: regression

BugBot [:suhaib / :marco/ :calixte]

Updated

•

5 years ago

Type: enhancement → defect

Patricia Lawless

Comment 13

•

5 years ago

jlast: any progress on the perf regression?

Flags: needinfo?(jlaster)

Jason Laster [:jlast]

Comment 14

•

5 years ago

Hi Patricia, we think it is an accounting change, but have not been able to confirm.

Flags: needinfo?(jlaster)

Jason Laster [:jlast]

Comment 15

•

5 years ago

I believe this was addressed.

Status: REOPENED → RESOLVED

Closed: 5 years ago → 5 years ago

Resolution: --- → INVALID

Marco Mucci [:MarcoM]

Updated

•

5 years ago

Whiteboard: [debugger-mvp]

Marco Mucci [:MarcoM]

Updated

•

5 years ago

Whiteboard: [debugger-mvp]

You need to log in before you can comment on or make changes to this bug.