Remove experimental SourceMapService (uses server-side source map support)

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Developer Tools
P3
normal
RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: jryans, Assigned: tromey)

Tracking

(Blocks: 2 bugs)

Trunk
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

11 months ago
In bug 670002, an experimental `SourceMapService` was added.

The current state:

* The service is off by default because of perf issues (bug 1289570)
* This service uses server-side source map support
* It's only used in the console (old and new)

We're working towards client-side source maps, so we'll eventually want to remove this approach.
(Assignee)

Comment 1

10 months ago
Let's not forget to remove the client side code that will be obsolete:
location-store.js and source-map-service.js.
Depends on: 1359144
(Assignee)

Comment 2

10 months ago
And resolveLocation in target.js, and the code in the tab actor, and likewise TabClient.
(Assignee)

Updated

10 months ago
Duplicate of this bug: 1177525
(Assignee)

Comment 4

10 months ago
It's unclear if some of the existing server-side code is still in use.
Certainly the style sheet code is.
I think it's advisable to start with what's obviously unused and then
delete more in a later bug.
Assignee: nobody → ttromey
Comment hidden (mozreview-request)
(Reporter)

Comment 6

10 months ago
mozreview-review
Comment on attachment 8867715 [details]
Bug 1349354 - remove now-unused source-map-service;

https://reviewboard.mozilla.org/r/139290/#review142596

Great, looks like you've removed everything that can be removed for now.  For the server-side source mapping support in general, I'd say that should remain in place until the old debugger is removed, but I'm sure other arguments could be made...
Attachment #8867715 - Flags: review?(jryans) → review+
(Assignee)

Comment 7

10 months ago
Yeah, from my reading it requires removing the old debugger (or updating that to the client-side service)
as well as parts of the inspector.
I'll file a follow-up.
(Assignee)

Updated

10 months ago
Blocks: 1364992
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 10

10 months ago
eslint found an unused variable.
(Assignee)

Comment 11

10 months ago
The source-map-url-service.js change was wrong - I think I misunderstood the situation when I wrote
that module initially, since what is really happening is that generatedUrl is first computed by
SpiderMonkey during parsing, then given to us via the actor.  This isn't something we'll ever be
removing.  So, my next patch will just update the comment there.
Comment hidden (mozreview-request)

Comment 13

10 months ago
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fcc8c00308a1
remove now-unused source-map-service; r=jryans

Comment 14

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fcc8c00308a1
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.