Closed Bug 1230299 Opened 4 years ago Closed 4 years ago

Removing "waiting for sources..." and don't fetch sources twice

Categories

(DevTools :: Debugger, defect)

40 Branch
defect
Not set

Tracking

(firefox45 fixed)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed

People

(Reporter: jlong, Assigned: jlong)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Not sure if anyone has opinions about this, ni? fitzgen just to get someone else's opinion before I do it.
Flags: needinfo?(nfitzgerald)
Blocks: 1230215
+1 as long as we don't get rid of the "no sources" message on documents without any JS.
Flags: needinfo?(nfitzgerald)
Summary: Don't show a "waiting for sources..." message in source listing → Removing "waiting for sources..." and don't fetch sources twice
Definitely still going to show "No sources". I'm also go ahead and remove the extraneous `loadSources` call on tab navigation here.
Attached patch 1230299.patch (obsolete) — Splinter Review
Attachment #8695875 - Flags: review?(ejpbruel)
Assignee: nobody → jlong
Status: NEW → ASSIGNED
Attached patch 1230299.patchSplinter Review
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)
Those tests had a bad parent commit. Rebased: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a8e21497c1d
https://hg.mozilla.org/mozilla-central/rev/6320b1962b04
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.