Retry idempotent methods on Chrome-Proxy: bypass

Retry idempotent methods on Chrome-Proxy: bypass

Requests using all idempotent HTTP methods (See RFC 2616) should be
retried after receiving a bypass message from the data reduction
proxy. Before this CL, only GET was being retried.

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

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

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

[bengr, 2013-12-14 00:20:36]

mef: everything
mmenke: rubber-stamp

[mmenke, 2013-12-17 15:39:19]

On 2013/12/16 18:59:52, bengr1 wrote:

Misha's now a net/ owner, so I'm not needed.

[mef, 2013-12-18 16:34:41]

https://codereview.chromium.org/99283006/diff/1/net/http/http_network_layer_u...
File net/http/http_network_layer_unittest.cc (right):

https://codereview.chromium.org/99283006/diff/1/net/http/http_network_layer_u...
net/http/http_network_layer_unittest.cc:364: const char* method;
why is method const char* but content std::string?

https://codereview.chromium.org/99283006/diff/1/net/http/http_network_transac...
File net/http/http_network_transaction.cc (right):

https://codereview.chromium.org/99283006/diff/1/net/http/http_network_transac...
net/http/http_network_transaction.cc:991: if (request_->method == "GET" ||
Is request_->method guaranteed to be upper case?

[bengr, 2013-12-18 19:39:46]

https://codereview.chromium.org/99283006/diff/1/net/http/http_network_layer_u...
File net/http/http_network_layer_unittest.cc (right):

https://codereview.chromium.org/99283006/diff/1/net/http/http_network_layer_u...
net/http/http_network_layer_unittest.cc:364: const char* method;
On 2013/12/18 16:34:41, mef wrote:
> why is method const char* but content std::string?

No good reason. Changed so both are std::string.

https://codereview.chromium.org/99283006/diff/1/net/http/http_network_transac...
File net/http/http_network_transaction.cc (right):

https://codereview.chromium.org/99283006/diff/1/net/http/http_network_transac...
net/http/http_network_transaction.cc:991: if (request_->method == "GET" ||
On 2013/12/18 16:34:41, mef wrote:
> Is request_->method guaranteed to be upper case?

Yes. From RFC 2616, sec 5.1.1 Method:

"The Method  token indicates the method to be performed on the resource
identified by the Request-URI. The method is case-sensitive."

[mef, 2013-12-18 20:36:23]

+ see lint errors.

https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_lay...
File net/http/http_network_layer_unittest.cc (right):

https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_lay...
net/http/http_network_layer_unittest.cc:167: 
nit: blank line

https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_tra...
File net/http/http_network_transaction.cc (right):

https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_tra...
net/http/http_network_transaction.cc:1013: request_->method == "OPTIONS" ||
I'd keep 'We don't want to resubmit a POST if the proxy took some action.'

[bengr, 2013-12-18 21:05:10]

I'm not seeing any lint errors. Did you run something to see them?

https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_lay...
File net/http/http_network_layer_unittest.cc (right):

https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_lay...
net/http/http_network_layer_unittest.cc:167: 
On 2013/12/18 20:36:23, mef wrote:
> nit: blank line

Done.

https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_tra...
File net/http/http_network_transaction.cc (right):

https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_tra...
net/http/http_network_transaction.cc:1013: request_->method == "OPTIONS" ||
On 2013/12/18 20:36:23, mef wrote:
> I'd keep 'We don't want to resubmit a POST if the proxy took some action.'

Done.

[mef, 2013-12-18 21:08:56]

On 2013/12/18 21:05:10, bengr1 wrote:
> I'm not seeing any lint errors. Did you run something to see them?
> 
>
https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_lay...
> File net/http/http_network_layer_unittest.cc (right):
> 
>
https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_lay...
> net/http/http_network_layer_unittest.cc:167: 
> On 2013/12/18 20:36:23, mef wrote:
> > nit: blank line
> 
> Done.
> 
>
https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_tra...
> File net/http/http_network_transaction.cc (right):
> 
>
https://codereview.chromium.org/99283006/diff/40001/net/http/http_network_tra...
> net/http/http_network_transaction.cc:1013: request_->method == "OPTIONS" ||
> On 2013/12/18 20:36:23, mef wrote:
> > I'd keep 'We don't want to resubmit a POST if the proxy took some action.'
> 
> Done.

https://codereview.chromium.org/lint_patch/issue99283006_40001_50001

[mef, 2013-12-18 21:16:02]

lgtm

[commit-bot: I haz the power, 2013-12-18 21:45:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bengr@chromium.org/99283006/80001

[commit-bot: I haz the power, 2013-12-19 00:13:12]

Change committed as 241714

[Ramya, 2013-12-19 02:58:28]

A revert of this CL has been created in
https://codereview.chromium.org/98693016/ by cramya@chromium.org.

The reason for reverting is: Broke clankium tot.

[commit-bot: I haz the power, 2013-12-19 17:40:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bengr@chromium.org/99283006/100001

[commit-bot: I haz the power, 2013-12-19 17:43:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bengr@chromium.org/99283006/120001

[commit-bot: I haz the power, 2013-12-19 20:11:17]

Change committed as 241927

[Ramya, 2013-12-19 22:17:57]

A revert of this CL has been created in
https://codereview.chromium.org/107073004/ by cramya@chromium.org.

The reason for reverting is: Broke Android bot again.

[commit-bot: I haz the power, 2013-12-19 23:06:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bengr@chromium.org/99283006/140001

[commit-bot: I haz the power, 2013-12-20 07:26:24]

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

[commit-bot: I haz the power, 2013-12-20 16:54:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bengr@chromium.org/99283006/140001

[commit-bot: I haz the power, 2013-12-20 18:59:06]

Change committed as 242128