Listen to OnNetworkChanged instead of OnIPAddressChanged

components/google/core/browser/google_url_tracker.cc

Index: components/google/core/browser/google_url_tracker.cc
diff --git a/components/google/core/browser/google_url_tracker.cc b/components/google/core/browser/google_url_tracker.cc
index 0864f6a73dabbb00eda6c5d0adc39e35ef8dae73..531d24c924f23b77b8f44daec785e8dbee5209bd 100644
--- a/components/google/core/browser/google_url_tracker.cc
+++ b/components/google/core/browser/google_url_tracker.cc
@@ -245,6 +245,10 @@ void GoogleURLTracker::StartFetchIfDesirable() {
   // Configure to max_retries at most kMaxRetries times for 5xx errors.
   static const int kMaxRetries = 5;
   fetcher_->SetMaxRetriesOn5xx(kMaxRetries);
+#if defined(OS_ANDROID)
+  // Configure to max_retries at most kMaxRetries times for network changes.
+  fetcher_->SetAutomaticallyRetryOnNetworkChanges(kMaxRetries);

[Peter Kasting, 2014/08/27 19:52:46]

Let me understand this.  When the network changes, OnIPAddressChanged() gets
called, which calls StartFetchIfDesirable(), but the fetch we subsequently
trigger here can still fail with ERR_NETWORK_CHANGED after that?  Why?

Also, why is this Android-specific?  Wouldn't we want this retry behavior on all
platforms?

[Jaekyun Seok (inactive), 2014/08/28 02:00:52]

Yes, that is correctly what happened on clank.
I confirmed that when this problem happened HostResolverImpl::OnDNSChanged() was
called right after GoogleURLTracker::OnIPAddressChanged() was called, and so
HostResolverImpl::OnDNSChanged() aborted GoogleURLTracker's request. But this
doesn't always happen, and so I think that this is timing problem.
Actually, I tested this only on clank, and so I applied this only on android to
avoid any regression on other platforms. But I believe that this isn't harmful
at all. I will remove if macro.

[Peter Kasting, 2014/08/28 03:28:19]

Hmm.  Here are two ideas for a possible better fix:
(1) Ensure that HostResolverImpl::OnDNSChanged() gets called before any
OnIPAddressChanged() methods get called, so no on tries to do network accesses
that will immediately get aborted.
(2) Have OnIPAddressChanged() post a task to do the network access, so we finish
calling all the various handlers before we try to do this.

(1) is better than (2), if it would work, since (2) is basically a hack.  Either
one seems better than the current fix, though, since the current fix doesn't
prevent us from doing extra work that will fail, it just retries if we do so.

The network stack folks should probably weigh in.

+#endif
 
   fetcher_->Start();
 }