Closed
Bug 1230299
Opened 7 years ago
Closed 7 years ago
Removing "waiting for sources..." and don't fetch sources twice
Categories
(DevTools :: Debugger, defect)
Tracking
(firefox45 fixed)
RESOLVED
FIXED
Firefox 45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: jlong, Assigned: jlong)
References
Details
Attachments
(1 file, 1 obsolete file)
15.48 KB,
patch
|
Details | Diff | Splinter Review |
The initial message in the source listing is "Waiting for sources..." and it waits for LOAD_SOURCES and either shows sources or if it is empty shows "no sources". The problem is that once we are connected, when you reload the page, you already get all the sources as individual `newSource` notifications. So when you reload, we are actually fetching all the sources *twice*, and because we aren't using a declarative UI, it is even adding them to the UI twice, but the second time it adds it is basically a noop (but it still has to touch the DOM). I've noticed this before and it makes loading a lot of sources on the page slow. The *only* reason we need to call `loadSources()` when you reload is because we need to see if we get an empty array back. If there are no sources, we will never get any `newSource` notifications. But we show a loading message by default, so we have to know when to switch it to the "no sources" message. I don't think the loading message has much utility. I say we just remove it because it's causing a lot of complexity. Then it'll only call `loadSources` when the debugger is opened, but never again, because all subsequent sources will come in through `newSource` notifications.
Assignee | ||
Comment 1•7 years ago
|
||
Not sure if anyone has opinions about this, ni? fitzgen just to get someone else's opinion before I do it.
Flags: needinfo?(nfitzgerald)
Comment 2•7 years ago
|
||
+1 as long as we don't get rid of the "no sources" message on documents without any JS.
Flags: needinfo?(nfitzgerald)
Assignee | ||
Updated•7 years ago
|
Summary: Don't show a "waiting for sources..." message in source listing → Removing "waiting for sources..." and don't fetch sources twice
Assignee | ||
Comment 3•7 years ago
|
||
Definitely still going to show "No sources". I'm also go ahead and remove the extraneous `loadSources` call on tab navigation here.
Assignee | ||
Comment 4•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8695875 -
Flags: review?(ejpbruel)
Assignee | ||
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=74721cad6afb
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jlong
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•7 years ago
|
||
Whoops forgot to update some of the tests. We no longer have a LOAD_SOURCES event because we don't know when to emit it now (because we don't explicitly fetch the sources when reloading). But that's fine because the `navigation` event comes in after all the `newSource` events have been fired so we can just wait on that.
Attachment #8695875 -
Attachment is obsolete: true
Attachment #8695875 -
Flags: review?(ejpbruel)
Assignee | ||
Comment 7•7 years ago
|
||
New try push w/fixed tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=64a5dd44e263
Assignee | ||
Comment 8•7 years ago
|
||
Those tests had a bad parent commit. Rebased: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a8e21497c1d
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6320b1962b04
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Updated•4 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•