[Translate] Don't try to translate when offline

components/translate/core/browser/translate_manager_unittest.cc

Index: components/translate/core/browser/translate_manager_unittest.cc
diff --git a/components/translate/core/browser/translate_manager_unittest.cc b/components/translate/core/browser/translate_manager_unittest.cc
index b416f36f8071ab75d9804ea2f001b6c874a718e7..3bdcd50ac3f4912af0966e05e9e7daa83aaf44b7 100644
--- a/components/translate/core/browser/translate_manager_unittest.cc
+++ b/components/translate/core/browser/translate_manager_unittest.cc
@@ -4,11 +4,19 @@
 
 #include "components/translate/core/browser/translate_manager.h"
 
+#include "base/test/histogram_tester.h"
 #include "build/build_config.h"
+#include "components/infobars/core/infobar.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/pref_registry/testing_pref_service_syncable.h"
+#include "components/translate/core/browser/mock_translate_driver.h"
+#include "components/translate/core/browser/translate_browser_metrics.h"
+#include "components/translate/core/browser/translate_client.h"
 #include "components/translate/core/browser/translate_download_manager.h"
 #include "components/translate/core/browser/translate_prefs.h"
+#include "components/translate/core/common/translate_pref_names.h"
+#include "net/base/network_change_notifier.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace translate {
@@ -23,13 +31,95 @@ const char* kLanguagePreferredLanguages = nullptr;
 #endif
 const char kAcceptLanguages[] = "intl.accept_languages";
 
-class TranslateManagerTest : public testing::Test {
+// Override NetworkChangeNotifier to simulate connection type changes for tests.

[hajimehoshi, 2016/05/26 06:46:59]

nit: Overrides?

[groby-ooo-7-16, 2016/05/28 00:59:37]

Done. Slightly reworded to still stay at the 80 char limit :)

+// TODO(groby): Combine with similar code in ResourceRequestAllowedNotifierTest.
+class TestNetworkChangeNotifier : public net::NetworkChangeNotifier {
+ public:
+  TestNetworkChangeNotifier()
+      : net::NetworkChangeNotifier(),
+        connection_type_to_return_(
+            net::NetworkChangeNotifier::CONNECTION_UNKNOWN) {}
+
+  // Simulates a change of the connection type to |type|. This will notify any
+  // objects that are NetworkChangeNotifiers.
+  void SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::ConnectionType type) {
+    connection_type_to_return_ = type;
+    net::NetworkChangeNotifier::NotifyObserversOfConnectionTypeChangeForTests(
+        connection_type_to_return_);
+    base::MessageLoop::current()->RunUntilIdle();
+  }
+
+  void SetOffline(bool offline) {

[hajimehoshi, 2016/05/26 06:46:59]

Hmm, a function taking a boolean value is a little confusing (especially
false/true for online/offline is counterintuitive). How about EnableNetwork /
DisableNetwork?

[groby-ooo-7-16, 2016/05/28 00:59:37]

Since it's not actually enabling any networks (or actually going on/offline),
SimulateOffline/SimulateOnline is hopefully clearer.

Done.

+    connection_type_to_return_ =
+        offline ? net::NetworkChangeNotifier::CONNECTION_NONE
+                : net::NetworkChangeNotifier::CONNECTION_UNKNOWN;
+  }
+
+ private:
+  ConnectionType GetCurrentConnectionType() const override {
+    return connection_type_to_return_;
+  }
+
+  // The currently simulated network connection type. If this is set to
+  // CONNECTION_NONE, then NetworkChangeNotifier::IsOffline will return true.
+  net::NetworkChangeNotifier::ConnectionType connection_type_to_return_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestNetworkChangeNotifier);
+};
+
+// TODO(groby): Combine with MockTranslateClient in TranslateUiDelegateTest.
+class MockTranslateClient : public TranslateClient {
+ public:
+  MockTranslateClient(TranslateDriver* driver, PrefService* prefs)
+      : driver_(driver), prefs_(prefs) {}
+
+  // TODO(groby): Does TranslateClient need a virtual dtor?

[hajimehoshi, 2016/05/26 06:46:59]

How will you check this?

[groby-ooo-7-16, 2016/05/28 00:59:37]

Declared it - ChromeTranslateClient already declares its dtor as override, so
clearly the missing dtor on the interface is a mistake.

Fixed that in https://codereview.chromium.org/2019883002

+  virtual ~MockTranslateClient() {}
+
+  TranslateDriver* GetTranslateDriver() { return driver_; }
+  PrefService* GetPrefs() { return prefs_; }
+
+  std::unique_ptr<TranslatePrefs> GetTranslatePrefs() {
+    return base::WrapUnique(new TranslatePrefs(prefs_, kAcceptLanguages,
+                                               kLanguagePreferredLanguages));
+  }
+  MOCK_METHOD0(GetTranslateAcceptLanguages, TranslateAcceptLanguages*());
+  MOCK_CONST_METHOD0(GetInfobarIconID, int());
+
+#if !defined(USE_AURA)
+  MOCK_CONST_METHOD1(CreateInfoBarMock,
+                     infobars::InfoBar*(TranslateInfoBarDelegate*));
+  std::unique_ptr<infobars::InfoBar> CreateInfoBar(
+      std::unique_ptr<TranslateInfoBarDelegate> delegate) const {
+    return base::WrapUnique(CreateInfoBarMock(std::move(delegate).get()));
+  }
+#endif
+
+  MOCK_METHOD5(ShowTranslateUI,
+               void(translate::TranslateStep,
+                    const std::string&,
+                    const std::string&,
+                    TranslateErrors::Type,
+                    bool));
+
+  MOCK_METHOD1(IsTranslatableURL, bool(const GURL&));
+  MOCK_METHOD1(ShowReportLanguageDetectionErrorUI,
+               void(const GURL& report_url));
+
+ private:
+  TranslateDriver* driver_;
+  PrefService* prefs_;
+};
+
+class TranslateManagerTest : public ::testing::Test {
  protected:
   TranslateManagerTest()
       : translate_prefs_(&prefs_,
                          kAcceptLanguages,
                          kLanguagePreferredLanguages),
-        manager_(TranslateDownloadManager::GetInstance()) {}
+        manager_(TranslateDownloadManager::GetInstance()),
+        mock_translate_client_(&driver_, &prefs_) {}
 
   void SetUp() override {
     // Ensure we're not requesting a server-side translate language list.
@@ -40,12 +130,23 @@ class TranslateManagerTest : public testing::Test {
                                           std::string());
 #endif
     TranslatePrefs::RegisterProfilePrefs(prefs_.registry());
+    // TODO(groby): Figure out RegisterProfilePrefs() should register this.
+    prefs_.registry()->RegisterBooleanPref(
+        prefs::kEnableTranslate, true,
+        user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
     manager_->ResetForTesting();
   }
   user_prefs::TestingPrefServiceSyncable prefs_;
+
+  // TODO(groby): request TranslatePrefs from |mock_translate_client_| instead.
   TranslatePrefs translate_prefs_;
   TranslateDownloadManager* manager_;
 
+  TestNetworkChangeNotifier network_notifier_;
+  testing::MockTranslateDriver driver_;
+  ::testing::NiceMock<MockTranslateClient> mock_translate_client_;

[hajimehoshi, 2016/05/26 06:46:59]

Unify ::testing or testing

[groby-ooo-7-16, 2016/05/28 00:59:37]

Heh - one is actually ::testing, and the other is translate::testing :) I've
prefixed the first one with translate to be clearer about that, even though this
code is in the translate namespace.

+  std::unique_ptr<TranslateManager> translate_manager_;
+
   void TearDown() override { manager_->ResetForTesting(); }
 };
 
@@ -92,4 +193,33 @@ TEST_F(TranslateManagerTest, GetTargetLanguageAcceptLangFallback) {
   EXPECT_EQ("en", TranslateManager::GetTargetLanguage(&translate_prefs_));
 }
 
+TEST_F(TranslateManagerTest, DontTranslateOffline) {
+  TranslateManager::SetIgnoreMissingKeyForTesting(true);
+  translate_manager_.reset(new translate::TranslateManager(
+      &mock_translate_client_, kAcceptLanguages));
+
+  // The test measures that the "Translate was disabled" exit can only be
+  // reached after the early-out tests including IsOffline() passed.
+  const char kMetricName[] = "Translate.InitiationStatus.v2";
+  base::HistogramTester histogram_tester;
+
+  prefs_.SetBoolean(prefs::kEnableTranslate, false);
+
+  translate_manager_->GetLanguageState().LanguageDetermined("de", true);
+
+  // In the offline case, Initiate will early-out before even hitting the API
+  // key test.
+  network_notifier_.SetOffline(true);
+  translate_manager_->InitiateTranslation("de");
+  histogram_tester.ExpectTotalCount(kMetricName, 0);
+
+  // In the online case, InitiateTranslation will proceed past early out tests.
+  network_notifier_.SetOffline(false);
+  translate_manager_->InitiateTranslation("de");
+  histogram_tester.ExpectUniqueSample(
+      kMetricName,
+      translate::TranslateBrowserMetrics::INITIATION_STATUS_DISABLED_BY_PREFS,
+      1);
+}
+
 }  // namespace translate