Closed Bug 767920 Opened 8 years ago Closed 8 years ago
Overlap tcp/ssl handshakes with typing in search box
From Firefox Bug 735543: > When the user focuses on the search box we can try and TCP and/or SSL connect to > the search provider if there isn't an existing connection already. That allows > the overlap of the connection to happen with the typing - making the connection > overhead more or less free. > > The average RTT is around 100ms (with a wide variation). That's 2 or 3 RTT saved > for SSL providers, 1 for TCP. So that's a significant latency improvement. Our code is pretty similar. The SeaMonkey equivalent is somewhere here: <http://hg.mozilla.org/comm-central/annotate/b3396b2cf032/suite/common/search/search.xml#l456> <http://mxr.mozilla.org/comm-central/source/suite/common/search/search.xml>
Hi! Thanks for your patch. A couple of notes: > + var connector = > + Services.io.QueryInterface(Components.interfaces.nsISpeculativeConnect); Suite wrapping style is something like: var connector = Services.io .QueryInterface(etc). Also, Suite style is for .XBL not to rely on externally defined Services so you need to do something like: var connector = Components.classes["@mozilla.org/network/io-service;1"] .getService(Components.interfaces.nsIIOService) .QueryInterface(Components.interfaces.nsISpeculativeConnect); or perhaps: var connector = Components.classes["@mozilla.org/network/io-service;1"] .getService(Components.interfaces.nsISpeculativeConnect);
Comment on attachment 637450 [details] [diff] [review] v2 Thanks! A couple of final nits: > + <handler event="focus"> > + <![CDATA[ The formatting in this file is rather inconsistent but the other handlers seem to use: <handler event="focus"><![CDATA[ Which actually matches your closing: > + ]]></handler> > + // Speculatively connect to the current engine's search URI (and > + // suggest URI, if different) to reduce request latency Period at the end of a sentence please. Note to Neil: Should we have a preference to control this with the default being off?
Attachment #637450 - Flags: review?(philip.chee) → feedback+
Comment on attachment 637752 [details] [diff] [review] v3 >+ var connector = Components.classes["@mozilla.org/network/io-service;1"] >+ .getService(Components.interfaces.nsIIOService) >+ .QueryInterface(Components.interfaces.nsISpeculativeConnect); Nit: No need to use both getService and QueryInterface, just getService(Components.interfaces.nsISpeculativeConnect) will do.
Attachment #637752 - Flags: review?(neil) → review+
Ekanan Ketunuti, Thank you for your patch!
> Created attachment 638071 [details] [diff] [review] > patch for check-in, r=neil This patch didn't apply because you edited it manually. I know it's tempting but please stick to hg qrefesh. I've fixed this for you before pushing.. Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/4b0c1a50b836
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
(In reply to Philip Chee from comment #9) > This patch didn't apply because you edited it manually. I know it's tempting > but please stick to hg qrefesh. Sorry about that.
Comment on attachment 638071 [details] [diff] [review] patch for check-in, r=neil >@@ -475,16 +475,35 @@ -475: Indicates the position in the original file ,16: Indicates the number of lines showing from the original file (in this case, 2x8 lines of context, as there are no deletions or holes) +475: Indicates the position in the updated file (always the same for the first hunk in that file) ,35: Indicates the number of lines showing from the updated file But you actually removed a line, so there are only 34 lines showing now ;-) Alternatively, editors exist that will update the patch lines for you.
wouldn't it be easier to just do hg qrefresh; hg export ?
I only use mq to fix up my tree after I lose a push race with multiple patches.
You need to log in before you can comment on or make changes to this bug.