Regression test for anti-DDoS bugs in ChromeNetworkDelegate.

Regression test for anti-DDoS bugs in ChromeNetworkDelegate.

This ensures:
a) That when g_never_throttle_requests_ is false, only
extension requests may be throttled (rather than the inverse);
b) That setting the flag applies to all instances of
ChromeNetworkDelegate, not just the one it is set on.

If we'd had these tests earlier, they would have prevented the
bugs below from being introduced.

TBR=thakis@chromium.org
BUG=141081, 141643
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=151683

[Jói, 2012-08-14 13:02:13]

Gentle ping.

[eroman, 2012-08-14 16:41:07]

http://codereview.chromium.org/10828284/diff/1/chrome/browser/net/chrome_netw...
File chrome/browser/net/chrome_network_delegate.h (right):

http://codereview.chromium.org/10828284/diff/1/chrome/browser/net/chrome_netw...
chrome/browser/net/chrome_network_delegate.h:12: #include
"base/gtest_prod_util.h"
Is this necessary?

http://codereview.chromium.org/10828284/diff/1/chrome/browser/net/chrome_netw...
File chrome/browser/net/chrome_network_delegate_unittest.cc (right):

http://codereview.chromium.org/10828284/diff/1/chrome/browser/net/chrome_netw...
chrome/browser/net/chrome_network_delegate_unittest.cc:18:
ChromeNetworkDelegate* CreateNetworkDelegate() {
[optional]: return a scoped_ptr<ChromeNetworkDelegate>

http://codereview.chromium.org/10828284/diff/1/chrome/browser/net/chrome_netw...
chrome/browser/net/chrome_network_delegate_unittest.cc:23: forwarder.get(),
NULL, NULL, NULL, NULL, &pref_member, NULL);
Is passing &pref_member safe? That variable will be deleted after returning...

Probably better to make CreateNetworkDelegate a member function and have the
dependency be scoped to the test fixture.

[Jói, 2012-08-14 22:17:10]

Please take another look.

Cheers,
Jói

http://codereview.chromium.org/10828284/diff/1/chrome/browser/net/chrome_netw...
File chrome/browser/net/chrome_network_delegate.h (right):

http://codereview.chromium.org/10828284/diff/1/chrome/browser/net/chrome_netw...
chrome/browser/net/chrome_network_delegate.h:12: #include
"base/gtest_prod_util.h"
On 2012/08/14 16:41:07, eroman wrote:
> Is this necessary?

Nope, thanks for catching that.  For a while I was using
FRIEND_TEST_ALL_PREFIXES from that header.

http://codereview.chromium.org/10828284/diff/1/chrome/browser/net/chrome_netw...
File chrome/browser/net/chrome_network_delegate_unittest.cc (right):

http://codereview.chromium.org/10828284/diff/1/chrome/browser/net/chrome_netw...
chrome/browser/net/chrome_network_delegate_unittest.cc:18:
ChromeNetworkDelegate* CreateNetworkDelegate() {
On 2012/08/14 16:41:07, eroman wrote:
> [optional]: return a scoped_ptr<ChromeNetworkDelegate>

Done.

http://codereview.chromium.org/10828284/diff/1/chrome/browser/net/chrome_netw...
chrome/browser/net/chrome_network_delegate_unittest.cc:23: forwarder.get(),
NULL, NULL, NULL, NULL, &pref_member, NULL);
On 2012/08/14 16:41:07, eroman wrote:
> Is passing &pref_member safe? That variable will be deleted after returning...
> 
> Probably better to make CreateNetworkDelegate a member function and have the
> dependency be scoped to the test fixture.

It's safe for now but the approach you suggested is less likely to break, so
better for maintenance.  Made this stuff into members.

[eroman, 2012-08-14 22:22:43]

lgtm

[commit-bot: I haz the power, 2012-08-14 22:23:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/10828284/5

[commit-bot: I haz the power, 2012-08-14 22:23:53]

Presubmit check for 10828284-5 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

[Jói, 2012-08-15 08:39:53]

TBRing to Nico for chrome/OWNERS, this is just a trivial addition of a source
file to the unit_tests target.

Cheers,
Jói

[commit-bot: I haz the power, 2012-08-15 08:43:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/10828284/5

[commit-bot: I haz the power, 2012-08-15 11:13:12]

Change committed as 151683

[Nico, 2012-08-15 17:27:15]

gentle lgtm