Implement support for QUIC proxies in ProxyServer

Implement support for QUIC proxies in ProxyServer

BUG=335275
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248904

[Ryan Hamilton, 2014-01-31 00:15:08]

As discussed earlier .... :>

(I'm working on a follow up CL to make use of this.)

[eroman, 2014-01-31 01:24:23]

Update GetDefaultPortForScheme()

In fact, you should probably delete the default handler so compiler warning
forces you to update it.

Lastly, please add a comment or link to documentation. a "quic proxy" is not
exactly a well understood concept :)

[Ryan Hamilton, 2014-01-31 18:03:46]

On 2014/01/31 01:24:23, eroman wrote:
> Update GetDefaultPortForScheme()

I think I did?

> In fact, you should probably delete the default handler so compiler warning
> forces you to update it.

Done, but I had to add a return at the end of the method or else one of our
compilers pukes.  (I think windows)/
 
> Lastly, please add a comment or link to documentation. a "quic proxy" is not
> exactly a well understood concept :)

Heh.  Done.

[eroman, 2014-01-31 18:49:51]

On Fri, Jan 31, 2014 at 10:03 AM, <rch@chromium.org> wrote:

> On 2014/01/31 01:24:23, eroman wrote:
>
>> Update GetDefaultPortForScheme()
>>
>
> I think I did?


Oops, indeed you had! Not sure what was wrong with my brain...


>
>
>  In fact, you should probably delete the default handler so compiler
>> warning
>> forces you to update it.
>>
>
> Done, but I had to add a return at the end of the method or else one of our
> compilers pukes.  (I think windows)/
>
>
>  Lastly, please add a comment or link to documentation. a "quic proxy" is
>> not
>> exactly a well understood concept :)
>>
>
> Heh.  Done.
>
>
> https://codereview.chromium.org/144623006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[eroman, 2014-01-31 18:51:00]

lgtm

[Ryan Hamilton, 2014-01-31 19:43:57]

battre: chrome/browser/extensions/api/proxy/proxy_api_helpers.cc

[battre, 2014-02-03 10:14:57]

https://codereview.chromium.org/144623006/diff/130001/chrome/browser/extensio...
File chrome/browser/extensions/api/proxy/proxy_api_helpers.cc (right):

https://codereview.chromium.org/144623006/diff/130001/chrome/browser/extensio...
chrome/browser/extensions/api/proxy/proxy_api_helpers.cc:441: break;
I think there is a little bit more work to the proxy settings API:

You need to extend the ProxyServer type to support the new scheme:
http://developer.chrome.com/extensions/proxy.html#type-ProxyServer

It would be great to cover this in a roundtrip test. Maybe you can take this
test:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/e...

And modify it as follows:
var httpProxy = {
  scheme: "quic",
  host: "1.1.1.1"
};
var httpProxyExpected = {
  scheme: "quic",
  host: "1.1.1.1",
  port: 80
};

Thanks.

[Ryan Hamilton, 2014-02-03 22:28:38]

https://codereview.chromium.org/144623006/diff/130001/chrome/browser/extensio...
File chrome/browser/extensions/api/proxy/proxy_api_helpers.cc (right):

https://codereview.chromium.org/144623006/diff/130001/chrome/browser/extensio...
chrome/browser/extensions/api/proxy/proxy_api_helpers.cc:441: break;
On 2014/02/03 10:14:57, battre wrote:
> I think there is a little bit more work to the proxy settings API:
> 
> You need to extend the ProxyServer type to support the new scheme:
> http://developer.chrome.com/extensions/proxy.html#type-ProxyServer
> 
> It would be great to cover this in a roundtrip test. Maybe you can take this
> test:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/e...
> 
> And modify it as follows:
> var httpProxy = {
>   scheme: "quic",
>   host: "1.1.1.1"
> };
> var httpProxyExpected = {
>   scheme: "quic",
>   host: "1.1.1.1",
>   port: 80
> };
> 
> Thanks.

Ah, I see.  Done.  (I hope)

[battre, 2014-02-04 08:46:55]

Proxy API code LGTM

[Ryan Hamilton, 2014-02-04 18:43:36]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2014-02-04 18:48:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/144623006/230001

[commit-bot: I haz the power, 2014-02-04 19:21:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-04 19:21:08]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Ryan Hamilton, 2014-02-05 00:00:16]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2014-02-05 00:12:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/144623006/230001

[commit-bot: I haz the power, 2014-02-05 01:46:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/144623006/230001

[commit-bot: I haz the power, 2014-02-05 04:24:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-05 04:24:14]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Ryan Hamilton, 2014-02-05 04:36:47]

kalman: proxy.json

[not at google - send to devlin, 2014-02-05 04:58:37]

lgtm

[Ryan Hamilton, 2014-02-05 05:01:28]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2014-02-05 05:06:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/144623006/230001

[commit-bot: I haz the power, 2014-02-05 07:38:08]

Change committed as 248904