767920 - Overlap tcp/ssl handshakes with typing in search box.

Status: RESOLVED FIXED

(SeaMonkey :: Search, defect)

Description

[Philip Chee]

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>

Comment 1

[Ekanan Ketunuti]

Attachment: v1

Comment 2

[Philip Chee]

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 3

[Ekanan Ketunuti]

Attachment: v2

Comment 4

[Philip Chee]

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?

Comment 5

[Ekanan Ketunuti]

Attachment: v3

Comment 6

[[e-mail]]

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.

Comment 7

[Ekanan Ketunuti]

Attachment: patch for check-in, r=neil

Comment 8

[Philip Chee]

Ekanan Ketunuti, Thank you for your patch!

Comment 9

[Philip Chee]

> 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

Comment 10

[Ekanan Ketunuti]

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

[[e-mail]]

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.

Comment 12

[Philip Chee]

wouldn't it be easier to just do hg qrefresh; hg export ?

Comment 13

[[e-mail]]

I only use mq to fix up my tree after I lose a push race with multiple patches.