[Translate] Don't try to translate when offline

[Translate] Don't try to translate when offline

If the user reloads a page during a network outage, and language
detection for this page has completed before, Chrome will currently
offer to translate that page, even though this cannot possibly succeed
when offline.

This CL adds an early-out for any attempts to translate content while
offline.

TEST=Load https://namu.wiki/w/나무위키:대문 - then turn off network. Hit reload. Observe translate is not offered a second time.

BUG=612973
Committed: https://crrev.com/0e7f1247be559c523607aa1452dfd2a351b85173
Cr-Commit-Position: refs/heads/master@{#396683}

[groby-ooo-7-16, 2016-05-19 05:40:45]

Description was changed from

==========
[Translate] Don't try to translate when offline

If the user reloads a page during a network outage, and language
detection for this page has completed before, Chrome will currently
offer to translate that page, even though this cannot possibly succeed
when offline.

This CL adds an early-out for any attempts to translate content while
offline.

TEST=Load https://namu.wiki/w/나무위키:대문 - then turn off network. Hit reload.
Observe translate is not offered a second time.

BUG=612973
==========

to

==========
[Translate] Don't try to translate when offline

If the user reloads a page during a network outage, and language
detection for this page has completed before, Chrome will currently
offer to translate that page, even though this cannot possibly succeed
when offline.

This CL adds an early-out for any attempts to translate content while
offline.

TEST=Load https://namu.wiki/w/나무위키:대문 - then turn off network. Hit reload.
Observe translate is not offered a second time.

BUG=612973
==========

[groby-ooo-7-16, 2016-05-19 05:40:45]

groby@chromium.org changed reviewers:
+ hajimehoshi@chromium.org

[groby-ooo-7-16, 2016-05-25 22:59:29]

hajimehoshi@: PTAL

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

Awesome!

https://codereview.chromium.org/1997573002/diff/80001/components/translate/co...
File components/translate/core/browser/translate_manager_unittest.cc (right):

https://codereview.chromium.org/1997573002/diff/80001/components/translate/co...
components/translate/core/browser/translate_manager_unittest.cc:34: // Override
NetworkChangeNotifier to simulate connection type changes for tests.
nit: Overrides?

https://codereview.chromium.org/1997573002/diff/80001/components/translate/co...
components/translate/core/browser/translate_manager_unittest.cc:53: void
SetOffline(bool offline) {
Hmm, a function taking a boolean value is a little confusing (especially
false/true for online/offline is counterintuitive). How about EnableNetwork /
DisableNetwork?

https://codereview.chromium.org/1997573002/diff/80001/components/translate/co...
components/translate/core/browser/translate_manager_unittest.cc:77: //
TODO(groby): Does TranslateClient need a virtual dtor?
How will you check this?

https://codereview.chromium.org/1997573002/diff/80001/components/translate/co...
components/translate/core/browser/translate_manager_unittest.cc:147:
::testing::NiceMock<MockTranslateClient> mock_translate_client_;
Unify ::testing or testing

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

https://codereview.chromium.org/1997573002/diff/80001/components/translate/co...
File components/translate/core/browser/translate_manager_unittest.cc (right):

https://codereview.chromium.org/1997573002/diff/80001/components/translate/co...
components/translate/core/browser/translate_manager_unittest.cc:34: // Override
NetworkChangeNotifier to simulate connection type changes for tests.
On 2016/05/26 06:46:59, hajimehoshi wrote:
> nit: Overrides?

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

https://codereview.chromium.org/1997573002/diff/80001/components/translate/co...
components/translate/core/browser/translate_manager_unittest.cc:53: void
SetOffline(bool offline) {
On 2016/05/26 06:46:59, hajimehoshi wrote:
> Hmm, a function taking a boolean value is a little confusing (especially
> false/true for online/offline is counterintuitive). How about EnableNetwork /
> DisableNetwork?

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

Done.

https://codereview.chromium.org/1997573002/diff/80001/components/translate/co...
components/translate/core/browser/translate_manager_unittest.cc:77: //
TODO(groby): Does TranslateClient need a virtual dtor?
On 2016/05/26 06:46:59, hajimehoshi wrote:
> How will you check this?

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

https://codereview.chromium.org/1997573002/diff/80001/components/translate/co...
components/translate/core/browser/translate_manager_unittest.cc:147:
::testing::NiceMock<MockTranslateClient> mock_translate_client_;
On 2016/05/26 06:46:59, hajimehoshi wrote:
> Unify ::testing or testing

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.

[hajimehoshi, 2016-05-30 04:41:28]

lgtm

[hajimehoshi, 2016-05-30 04:41:31]

The CQ bit was checked by hajimehoshi@chromium.org

[commit-bot: I haz the power, 2016-05-30 04:41:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1997573002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1997573002/100001

[commit-bot: I haz the power, 2016-05-30 05:47:25]

Description was changed from

==========
[Translate] Don't try to translate when offline

If the user reloads a page during a network outage, and language
detection for this page has completed before, Chrome will currently
offer to translate that page, even though this cannot possibly succeed
when offline.

This CL adds an early-out for any attempts to translate content while
offline.

TEST=Load https://namu.wiki/w/나무위키:대문 - then turn off network. Hit reload.
Observe translate is not offered a second time.

BUG=612973
==========

to

==========
[Translate] Don't try to translate when offline

If the user reloads a page during a network outage, and language
detection for this page has completed before, Chrome will currently
offer to translate that page, even though this cannot possibly succeed
when offline.

This CL adds an early-out for any attempts to translate content while
offline.

TEST=Load https://namu.wiki/w/나무위키:대문 - then turn off network. Hit reload.
Observe translate is not offered a second time.

BUG=612973
==========

[commit-bot: I haz the power, 2016-05-30 05:47:26]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-05-30 05:48:57]

Description was changed from

==========
[Translate] Don't try to translate when offline

If the user reloads a page during a network outage, and language
detection for this page has completed before, Chrome will currently
offer to translate that page, even though this cannot possibly succeed
when offline.

This CL adds an early-out for any attempts to translate content while
offline.

TEST=Load https://namu.wiki/w/나무위키:대문 - then turn off network. Hit reload.
Observe translate is not offered a second time.

BUG=612973
==========

to

==========
[Translate] Don't try to translate when offline

If the user reloads a page during a network outage, and language
detection for this page has completed before, Chrome will currently
offer to translate that page, even though this cannot possibly succeed
when offline.

This CL adds an early-out for any attempts to translate content while
offline.

TEST=Load https://namu.wiki/w/나무위키:대문 - then turn off network. Hit reload.
Observe translate is not offered a second time.

BUG=612973

Committed: https://crrev.com/0e7f1247be559c523607aa1452dfd2a351b85173
Cr-Commit-Position: refs/heads/master@{#396683}
==========

[commit-bot: I haz the power, 2016-05-30 05:48:57]

Patchset 6 (id:??) landed as
https://crrev.com/0e7f1247be559c523607aa1452dfd2a351b85173
Cr-Commit-Position: refs/heads/master@{#396683}