Add a command line switch "--disable-background-networking", to be used in...

Add a command line switch "--disable-background-networking", to be used in
benchmarking when unexpected background networking can cause undesired vairance.

The following systems are disabled via this flag:
   - IntranetRedirectDetector (requests randomURLs 2-5s after startup)
   - GoogleUrlTracker (searchdomaincheck)
   - SafeBrowsing updater
   - Extension updater

BUG=none
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=58892

[Mike Belshe, 2010-09-07 18:23:11]

Antony - most of this change is generic and not related to extensions.  Sent to
you because it touches the extension updater.  Hopefully I put the escape hatch
in the right place.

Just a debugging cmdline switch.

[Mike Belshe, 2010-09-08 19:32:49]

Since Antony is out...  Maybe Peter can take a look...  thanks!

[Peter Kasting, 2010-09-08 19:38:44]

LGTM

http://codereview.chromium.org/3312014/diff/1/3
File chrome/browser/google/google_url_tracker.cc (right):

http://codereview.chromium.org/3312014/diff/1/3#newcode193
chrome/browser/google/google_url_tracker.cc:193: if
(CommandLine::ForCurrentProcess()->HasSwitch(
Nit: I'd put this at the end of the above conditional list, or else at the top
of the function.

http://codereview.chromium.org/3312014/diff/1/4
File chrome/browser/intranet_redirect_detector.cc (right):

http://codereview.chromium.org/3312014/diff/1/4#newcode83
chrome/browser/intranet_redirect_detector.cc:83: if
(CommandLine::ForCurrentProcess()->HasSwitch(
Nit: I'd put this at the end of the above conditional list, or else at the top
of the function.

[Mike Belshe, 2010-09-08 22:13:45]

Thanks - I checked with another person for a vote.  Decided to leave it with the
current ordering.

[cbentzel, 2010-09-08 22:19:31]

On 2010/09/08 22:13:45, Mike Belshe wrote:
> Thanks - I checked with another person for a vote.  Decided to leave it with
the
> current ordering.

Would it make sense to just disable at the UrlFetcher level if this is for
debugging only?

[Peter Kasting, 2010-09-08 22:23:46]

On 2010/09/08 22:19:31, cbentzel wrote:
> Would it make sense to just disable at the UrlFetcher level if this is for
> debugging only?

I suggested that but Mike didn't want to.

[mbelshe, 2010-09-08 23:47:58]

On Wed, Sep 8, 2010 at 3:23 PM, <pkasting@chromium.org> wrote:

> On 2010/09/08 22:19:31, cbentzel wrote:
>
>> Would it make sense to just disable at the UrlFetcher level if this is for
>> debugging only?
>>
>
> I suggested that but Mike didn't want to.


I was trying to leave as much in tact as possible; just extracting the
network requests.

Is it true that URLFetcher is only used for non-essential background
networking tasks and will never be used for anything else?

If so, then maybe this would be okay.

Mike


>
>
> http://codereview.chromium.org/3312014/show
>

[Peter Kasting, 2010-09-09 01:14:44]

On 2010/09/08 23:47:58, mbelshe wrote:
> Is it true that URLFetcher is only used for non-essential background
> networking tasks and will never be used for anything else?

No.  I misread cbentzel's request and mentally inserted the various URLFetcher
consumers instead.  URLFetcher itself can't be disabled under this flag.

[cbentzel, 2010-09-09 02:22:22]

Sometimes when I'm debugging network code I modify URLFetcher::Core::Start
to early return and skip the post of StartURLRequest callback.

This isn't the most disciplined approach, but it helps minimize the
non-explicit requests. Doing something similar with a command line flag
might make sense.

On Wed, Sep 8, 2010 at 9:14 PM, <pkasting@chromium.org> wrote:

> On 2010/09/08 23:47:58, mbelshe wrote:
>
>> Is it true that URLFetcher is only used for non-essential background
>> networking tasks and will never be used for anything else?
>>
>
> No.  I misread cbentzel's request and mentally inserted the various
> URLFetcher
> consumers instead.  URLFetcher itself can't be disabled under this flag.
>
>
> http://codereview.chromium.org/3312014/show
>

[asargent_no_longer_on_chrome, 2010-09-09 17:52:49]

It would be nice to have one common code path (a BackgroundUrlFetcher
class?) for all background-fetching code for things like honoring "please
backoff" response codes from servers, and other speculative ideas like a
mode for "I'm on expensive cell phone tethering; please minimize unnecessary
requests".

I know that Joi had an intern working on something like this but am not sure
if any of it got committed.


On Tue, Sep 7, 2010 at 11:23 AM, <mbelshe@chromium.org> wrote:

> Reviewers: Antony Sargent,
>
> Message:
> Antony - most of this change is generic and not related to extensions.
>  Sent to
> you because it touches the extension updater.  Hopefully I put the escape
> hatch
> in the right place.
>
> Just a debugging cmdline switch.
>
> Description:
> Add a command line switch "--disable-background-networking", to be used in
> benchmarking when unexpected background networking can cause undesired
> vairance.
>
> The following systems are disabled via this flag:
>   - IntranetRedirectDetector (requests randomURLs 2-5s after startup)
>   - GoogleUrlTracker (searchdomaincheck)
>   - SafeBrowsing updater
>   - Extension updater
>
> BUG=none
> TEST=none
>
>
> Please review this at http://codereview.chromium.org/3312014/show
>
> SVN Base: svn://chrome-svn/chrome/trunk/src/
>
> Affected files:
>  M     chrome/browser/extensions/extension_updater.cc
>  M     chrome/browser/google/google_url_tracker.cc
>  M     chrome/browser/intranet_redirect_detector.cc
>  M     chrome/browser/safe_browsing/safe_browsing_service.cc
>  M     chrome/common/chrome_switches.h
>  M     chrome/common/chrome_switches.cc
>
>
>

[Peter Kasting, 2010-09-09 18:04:05]

On 2010/09/09 17:52:49, Antony Sargent wrote:
> It would be nice to have one common code path (a BackgroundUrlFetcher
> class?) for all background-fetching code for things like honoring "please
> backoff" response codes from servers,

We already honor backoff through a common codepath.

> and other speculative ideas like a
> mode for "I'm on expensive cell phone tethering; please minimize unnecessary
> requests".

It can be very difficult to determine which requests are "unnecessary" in this
sense.  I think it might be unwise to add separate APIs for these cases; it's
too easy to get their usage wrong.

This patch does what Mike wants.  Let's leave it at that.