ResetNTP/Overlay on connection status change.

Added InstantController as NetworkChangeObserver. I considered introducing an inner class InstantNetworkChangeObserver, but didn't see much harm in having InstantController be NetworkChangeObserver in addition to NotificationObserver.

Fake NetworkChangeNotifier for tests to manipulate network state.
Extend InstantExtendedTest to InstantExtendedNetworkTest to setup FakeNetworkChangeNotifier.

Open consideration - What is the right place to register InstantController as NetworkChangeObserver.

BUG=229170,238862

Review URL: https://chromiumcodereview.appspot.com/14607019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@209479 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 152 insertions, 1 deletions

diff --git a/chrome/browser/ui/browser_instant_controller.cc b/chrome/browser/ui/browser_instant_controller.cc
index 8832b7d..aa8b59d 100644
--- a/chrome/browser/ui/browser_instant_controller.cc
+++ b/chrome/browser/ui/browser_instant_controller.cc
@@ -60,10 +60,12 @@ BrowserInstantController::BrowserInstantController(Browser* browser)
                  base::Unretained(this)));
   ResetInstant(std::string());
   browser_->search_model()->AddObserver(this);
+  net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
 }
 
 BrowserInstantController::~BrowserInstantController() {
   browser_->search_model()->RemoveObserver(this);
+  net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
 }
 
 bool BrowserInstantController::MaybeSwapInInstantNTPContents(
@@ -273,6 +275,15 @@ void BrowserInstantController::ModelChanged(
     instant_.InstantSupportChanged(new_state.instant_support);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// BrowserInstantController, net::NetworkChangeNotifier::NetworkChangeObserver
+// implementation:
+
+void BrowserInstantController::OnNetworkChanged(
+    net::NetworkChangeNotifier::ConnectionType type) {
+  instant_.OnNetworkChanged(type);
+}
+
 void BrowserInstantController::OnDefaultSearchProviderChanged(
     const std::string& pref_name) {
   DCHECK_EQ(pref_name, std::string(prefs::kDefaultSearchProviderID));
diff --git a/chrome/browser/ui/browser_instant_controller.h b/chrome/browser/ui/browser_instant_controller.h
index a7ecae9..0ffaf10 100644
--- a/chrome/browser/ui/browser_instant_controller.h
+++ b/chrome/browser/ui/browser_instant_controller.h
@@ -13,6 +13,7 @@
 #include "chrome/browser/ui/search/instant_controller.h"
 #include "chrome/browser/ui/search/instant_unload_handler.h"
 #include "chrome/browser/ui/search/search_model_observer.h"
+#include "net/base/network_change_notifier.h"
 #include "ui/base/window_open_disposition.h"
 
 class Browser;
@@ -27,7 +28,9 @@ namespace gfx {
 class Rect;
 }
 
-class BrowserInstantController : public SearchModelObserver {
+class BrowserInstantController
+    : public SearchModelObserver,
+      public net::NetworkChangeNotifier::NetworkChangeObserver {
  public:
   explicit BrowserInstantController(Browser* browser);
   virtual ~BrowserInstantController();
@@ -114,6 +117,10 @@ class BrowserInstantController : public SearchModelObserver {
   // ensures that they are reloaded in a non-privileged renderer process.
   void OnDefaultSearchProviderChanged(const std::string& pref_name);
 
+  // Overridden from net::NetworkChangeNotifier::NetworkChangeObserver:
+  virtual void OnNetworkChanged(net::NetworkChangeNotifier::ConnectionType type)
+      OVERRIDE;
+
   // Replaces the contents at tab |index| with |new_contents| and deletes the
   // existing contents.
   void ReplaceWebContentsAt(int index,
diff --git a/chrome/browser/ui/search/instant_controller.cc b/chrome/browser/ui/search/instant_controller.cc
index 6e8ae04..cae9606 100644
--- a/chrome/browser/ui/search/instant_controller.cc
+++ b/chrome/browser/ui/search/instant_controller.cc
@@ -1197,6 +1197,17 @@ InstantNTP* InstantController::ntp() const {
   return ntp_.get();
 }
 
+void InstantController::OnNetworkChanged(
+    net::NetworkChangeNotifier::ConnectionType type) {
+  // Not interested in events conveying change to offline
+  if (type == net::NetworkChangeNotifier::CONNECTION_NONE)
+    return;
+  if (!extended_enabled_ || use_local_page_only_)
+    return;
+  if (!ntp_ || ntp_->IsLocal())
+    ResetNTP(GetInstantURL());
+}
+
 // TODO(shishir): We assume that the WebContent's current RenderViewHost is the
 // RenderViewHost being created which is not always true. Fix this.
 void InstantController::InstantPageRenderViewCreated(
diff --git a/chrome/browser/ui/search/instant_controller.h b/chrome/browser/ui/search/instant_controller.h
index 2a69e25..4806d73 100644
--- a/chrome/browser/ui/search/instant_controller.h
+++ b/chrome/browser/ui/search/instant_controller.h
@@ -26,6 +26,7 @@
 #include "chrome/common/search_types.h"
 #include "content/public/common/page_transition_types.h"
 #include "googleurl/src/gurl.h"
+#include "net/base/network_change_notifier.h"
 #include "ui/base/window_open_disposition.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/rect.h"
@@ -168,6 +169,9 @@ class InstantController : public InstantPage::Delegate,
   // CommitInstant() on the browser, and returns true. Else, returns false.
   bool CommitIfPossible(InstantCommitType type);
 
+  // If the network status changes, try to reset NTP and Overlay.
+  void OnNetworkChanged(net::NetworkChangeNotifier::ConnectionType type);
+
   // Called to indicate that the omnibox focus state changed with the given
   // |reason|. If |focus_state| is FOCUS_NONE, |view_gaining_focus| is set to
   // the view gaining focus.
@@ -283,6 +287,8 @@ class InstantController : public InstantPage::Delegate,
   FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, ProcessIsolation);
   FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, UnrelatedSiteInstance);
   FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, OnDefaultSearchProviderChanged);
+  FRIEND_TEST_ALL_PREFIXES(InstantExtendedNetworkTest,
+                           NTPReactsToNetworkChanges);
   FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest,
                            AcceptingURLSearchDoesNotNavigate);
   FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest, AcceptingJSSearchDoesNotRunJS);
diff --git a/chrome/browser/ui/search/instant_extended_interactive_uitest.cc b/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
index 804621e..1f715d8 100644
--- a/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
+++ b/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
@@ -9,6 +9,7 @@
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/prefs/pref_service.h"
+#include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -74,6 +75,7 @@
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "grit/generated_resources.h"
+#include "net/base/network_change_notifier.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -112,6 +114,26 @@ class QuittingHistoryDBTask : public history::HistoryDBTask {
   DISALLOW_COPY_AND_ASSIGN(QuittingHistoryDBTask);
 };
 
+class FakeNetworkChangeNotifier : public net::NetworkChangeNotifier {
+ public:
+  FakeNetworkChangeNotifier() : connection_type_(CONNECTION_NONE) {}
+
+  virtual ConnectionType GetCurrentConnectionType() const OVERRIDE {
+    return connection_type_;
+  }
+
+  void SetConnectionType(ConnectionType type) {
+    connection_type_ = type;
+    NotifyObserversOfNetworkChange(type);
+    base::RunLoop().RunUntilIdle();
+  }
+
+  virtual ~FakeNetworkChangeNotifier() {}
+
+ private:
+  ConnectionType connection_type_;
+  DISALLOW_COPY_AND_ASSIGN(FakeNetworkChangeNotifier);
+};
 }  // namespace
 
 class InstantExtendedTest : public InProcessBrowserTest,
@@ -250,6 +272,29 @@ class InstantExtendedTest : public InProcessBrowserTest,
   int on_toggle_voice_search_calls_;
 };
 
+class InstantExtendedNetworkTest : public InstantExtendedTest {
+ protected:
+  virtual void SetUpOnMainThread() OVERRIDE {
+    disable_for_test_.reset(new net::NetworkChangeNotifier::DisableForTest);
+    fake_network_change_notifier_.reset(new FakeNetworkChangeNotifier);
+    InstantExtendedTest::SetUpOnMainThread();
+  }
+
+  virtual void CleanUpOnMainThread() OVERRIDE {
+    InstantExtendedTest::CleanUpOnMainThread();
+    fake_network_change_notifier_.reset();
+    disable_for_test_.reset();
+  }
+
+  void SetConnectionType(net::NetworkChangeNotifier::ConnectionType type) {
+    fake_network_change_notifier_->SetConnectionType(type);
+  }
+
+ private:
+  scoped_ptr<net::NetworkChangeNotifier::DisableForTest> disable_for_test_;
+  scoped_ptr<FakeNetworkChangeNotifier> fake_network_change_notifier_;
+};
+
 // Test class used to verify chrome-search: scheme and access policy from the
 // Instant overlay.  This is a subclass of |ExtensionBrowserTest| because it
 // loads a theme that provides a background image.
@@ -304,7 +349,73 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, NTPIsPreloaded) {
   content::WebContents* ntp_contents = instant()->ntp_->contents();
   EXPECT_TRUE(ntp_contents);
 }
+#endif  // HTML_INSTANT_EXTENDED_POPUP
+
+IN_PROC_BROWSER_TEST_F(InstantExtendedNetworkTest, NTPReactsToNetworkChanges) {
+  // Setup Instant.
+  ASSERT_NO_FATAL_FAILURE(SetupInstant(browser()));
+  FocusOmniboxAndWaitForInstantNTPSupport();
+
+  // The setup first initializes the platform specific NetworkChangeNotifier.
+  // The InstantExtendedNetworkTest replaces it with a fake, but by the time,
+  // instant controller has already registered itself. So the instant controller
+  // needs to register itself as NetworkChangeObserver again.
+  net::NetworkChangeNotifier::AddNetworkChangeObserver(browser_instant());
+
+  // The fake network change notifier will provide the network state to be
+  // offline, so the ntp will be local.
+  ASSERT_NE(static_cast<InstantNTP*>(NULL), instant()->ntp());
+  EXPECT_TRUE(instant()->ntp()->IsLocal());
+
+  // Change the connect state, and wait for the notifications to be run, and NTP
+  // support to be determined.
+  SetConnectionType(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  FocusOmniboxAndWaitForInstantNTPSupport();
+
+  // Verify the network state is fine, and instant controller doesn't want to
+  // switch to local NTP anymore.
+  EXPECT_FALSE(net::NetworkChangeNotifier::IsOffline());
+  EXPECT_FALSE(instant()->ShouldSwitchToLocalNTP());
 
+  // Open new tab.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      GURL(chrome::kChromeUINewTabURL),
+      NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
+  content::WebContents* active_tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Verify new NTP is not local.
+  EXPECT_TRUE(chrome::IsInstantNTP(active_tab));
+  EXPECT_NE(instant()->GetLocalInstantURL(), active_tab->GetURL().spec());
+  ASSERT_NE(static_cast<InstantNTP*>(NULL), instant()->ntp());
+  EXPECT_FALSE(instant()->ntp()->IsLocal());
+
+  SetConnectionType(net::NetworkChangeNotifier::CONNECTION_NONE);
+  FocusOmniboxAndWaitForInstantNTPSupport();
+
+  // Verify the network state is fine, and instant controller doesn't want to
+  // switch to local NTP anymore.
+  EXPECT_TRUE(net::NetworkChangeNotifier::IsOffline());
+  EXPECT_TRUE(instant()->ShouldSwitchToLocalNTP());
+
+  // Open new tab. Preloaded NTP contents should have been used.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      GURL(chrome::kChromeUINewTabURL),
+      NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
+  active_tab = browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Verify new NTP is not local.
+  EXPECT_TRUE(chrome::IsInstantNTP(active_tab));
+  EXPECT_EQ(instant()->GetLocalInstantURL(), active_tab->GetURL().spec());
+  ASSERT_NE(static_cast<InstantNTP*>(NULL), instant()->ntp());
+  EXPECT_TRUE(instant()->ntp()->IsLocal());
+}
+
+#if defined(HTML_INSTANT_EXTENDED_POPUP)
 IN_PROC_BROWSER_TEST_F(InstantExtendedTest, PreloadedNTPIsUsedInSameTab) {
   // Setup Instant.
   ASSERT_NO_FATAL_FAILURE(SetupInstant(browser()));
diff --git a/chrome/browser/ui/search/instant_test_utils.h b/chrome/browser/ui/search/instant_test_utils.h
index c07026a..1f0ce7a 100644
--- a/chrome/browser/ui/search/instant_test_utils.h
+++ b/chrome/browser/ui/search/instant_test_utils.h
@@ -21,6 +21,7 @@
 #include "googleurl/src/gurl.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 
+class BrowserInstantController;
 class InstantController;
 class InstantModel;
 class OmniboxView;
@@ -73,6 +74,10 @@ class InstantTestBase {
     browser_ = browser;
   }
 
+  BrowserInstantController* browser_instant() {
+    return browser_->instant_controller();
+  }
+
   InstantController* instant() {
     return browser_->instant_controller()->instant();
   }