Closed Bug 581978 Opened 10 years ago Closed 6 years ago

URL should load immediately after pressing enter

Categories

(Firefox :: Address Bar, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: jrmuizel, Unassigned)

References

Details

(Keywords: perf)

Currently it seems like we do a bunch of stuff that isn't relevant to the process of loading the page. It would be nice if we just loaded the page immediately. This is very apparent when editing data urls.
Fixing Bug 462690 (what is about Typing + Hitting "Enter" too) might help here if i get your Issue right.
Keywords: perf
It would be good to get some timing information to be able to actually quantify the delay and perhaps profile it. Does anyone know the right places to hook this in?
Maybe call sharkStart() on the Enter keypress event and sharkStop() when the tab navigation has begun (I'm forgetting which notification that was).
You want to sit down tomorrow at the office and do this?  Shouldn't take more than 10-15 minutes to write the patch...
(In reply to comment #4)
> You want to sit down tomorrow at the office and do this?  Shouldn't take more
> than 10-15 minutes to write the patch...

Sounds wonderful.
> Does anyone know the right places to hook this in?

The stop signal can go in the page itself, right?  In particular, in a shark build you can just do:

  data:text/html,<script>stopShark(); disconnectShark()</script>

You'll need OS X 10.5, of course.

The start signal, I had no idea.  But setting a breakpoint in nsDocShell::InternalLoad and examining the callstacks (C++ and JS) suggests that the right place is probably the onKeyPress function in autocomplete.xml around the handleEnter call.  Unless we think the time might be taken during the event dispatch before then...
fwiw, over here a Shark profile sampling every 20us shows 2300 samples or so the first time I try that, and 1160 samples thereafter.  That's 46ms delay the first time and 23ms after that.  About 2/3 of that time is painting.
I'm seeing sqlite3_step take >500ms on a background thread, while on the main thread there's a stack involving nsAnnotationService::GetPageAnnotationString.  It looks like lock contention -- bug 563538 -- and sdwilsh confirms over IRC.  With a new profile I didn't notice any lags, but when I profiled with my own profile, which has lots of history naturally, I was able to record this.
Depends on: 563538
Hmm.  Who's calling GetPageAnnotationString?
Stack looks like this:

nsThread::ProcessNextEvent
nsInputStreamReadyEvent::Run
nsInputStreamPump::OnInputStreamReady
nsBaseChannel::OnStartRequest
nsDocumentOpenInfo::OnStartRequest
nsDocumentOpenInfo::DispatchContent
nsDocumentOpenInfo::TryContentListener
nsDSURIContentListener::DoContent
nsDocShell::CreateContentViewer
nsDocShell::NewContentViewerObj
nsContentDLF::CreateInstance
nsContentDLF::CreateDocument
nsHTMLDocument::StartDocumentLoad
nsHTMLDocument::TryBookmarkCharset
nsNavHistory::RequestCharset
nsNavHistory::GetCharsetForURI
nsAnnotationService::GetPageAnnotationString
Not directly related to this bug, I suppose, but can we just kill TryBookmarkCharset? I wonder whether it's actually useful enough to justify extracting data from places at that stage of the loading process.
It would break some mislabeled pages that users have bookmarked and used "view > charset" on; past that, probably not a problem to remove it...
Depends on: 582703
Bug 572030 may possibly help here as well.
Depends on: 572030
(In reply to comment #11)
> can we just kill TryBookmarkCharset?

I filed bug 582712.
Any progress or change on this?
(In reply to comment #15)
> Any progress or change on this?

All dependencies are fixed in 4.0, so it should have improved, whether it is fixed or not it's hard to tell without a goal.
(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #16)
> (In reply to comment #15)
> > Any progress or change on this?
> 
> All dependencies are fixed in 4.0, so it should have improved, whether it is
> fixed or not it's hard to tell without a goal.

One measure might be if it meets Jeff's expectations?
WFM?
Flags: needinfo?(jmuizelaar)
I think regardless, the code is now so different that at a maximum we'd need a new bug with a new investigation.
But we didn't get reports of this recently, afaik.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
Flags: needinfo?(jmuizelaar)
You need to log in before you can comment on or make changes to this bug.