581978 - URL should load immediately after pressing enter

Status: RESOLVED WORKSFORME

(Firefox :: Address Bar, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

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.

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

Fixing Bug 462690 (what is about Typing + Hitting "Enter" too) might help here if i get your Issue right.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

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?

Comment 3

[(no longer active)]

Maybe call sharkStart() on the Enter keypress event and sharkStop() when the tab navigation has begun (I'm forgetting which notification that was).

Comment 4

[(no longer active)]

You want to sit down tomorrow at the office and do this?  Shouldn't take more than 10-15 minutes to write the patch...

Comment 5

[Jeff Muizelaar [:jrmuizel]]

(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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> 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...

Comment 7

[Boris Zbarsky [:bzbarsky]]

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.

Comment 8

[Drew Willcoxon :adw]

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.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Hmm.  Who's calling GetPageAnnotationString?

Comment 10

[Drew Willcoxon :adw]

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

Comment 11

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 12

[Boris Zbarsky [:bzbarsky]]

It would break some mislabeled pages that users have bookmarked and used "view > charset" on; past that, probably not a problem to remove it...

Comment 13

[(no longer active)]

Bug 572030 may possibly help here as well.

Comment 14

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #11)
> can we just kill TryBookmarkCharset?

I filed bug 582712.

Comment 15

[Tomas]

Any progress or change on this?

Comment 16

[Marco Bonardo [:mak]]

(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.

Comment 17

[Wayne Mery (:wsmwk)]

(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?

Comment 18

[Marco Bonardo [:mak]]

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.