Fix the problem that offline indicator is shown when it should not.

Two problems are fixed:
1) Treat the app as still in foreground state when the app state is
HAS_PAUSED_ACTIVITIES because this can happen when the activity is
temporarily covered by another activity.
2) Detecting continuous online should be checked when switching
from online to offline.

Bug: 903649
Change-Id: Icbb2fe4b1021d9525940472e33ae820ca3117667
Reviewed-on: https://chromium-review.googlesource.com/c/1330705Reviewed-by: Peter Williamson <petewil@chromium.org>
Commit-Queue: Jian Li <jianli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607045}

chrome/android/java/src/org/chromium/chrome/browser/offlinepages/indicator/OfflineIndicatorController.java

@@ -61,6 +61,8 @@ public class OfflineIndicatorController implements ConnectivityDetector.Observer
     private boolean mIsShowingOfflineIndicator;
     // Set to true if the offline indicator has been shown once since the activity has resumed.
     private boolean mHasOfflineIndicatorShownSinceActivityResumed;
+    // Set to true if the user has been continuously online for the required duration.
+    private boolean mWasOnlineForRequiredDuration;
     private ConnectivityDetector mConnectivityDetector;
     private ChromeActivity mObservedActivity;
 
@@ -120,9 +122,14 @@ public class OfflineIndicatorController implements ConnectivityDetector.Observer
 
     @Override
     public void onApplicationStateChange(int newState) {
+        // Note that the paused state can happen when the activity is temporarily covered by another
+        // activity's Fragment, in which case we should still treat the app as in foreground.
+        if (newState != ApplicationState.HAS_RUNNING_ACTIVITIES
+                && newState != ApplicationState.HAS_PAUSED_ACTIVITIES) {
+            mHasOfflineIndicatorShownSinceActivityResumed = false;
+        }
         // If the application is resumed, update the connection state and show indicator if needed.
         if (newState == ApplicationState.HAS_RUNNING_ACTIVITIES) {
-            mHasOfflineIndicatorShownSinceActivityResumed = false;
             mConnectivityDetector.detect();
             updateOfflineIndicator(mConnectivityDetector.getConnectionState()
                     == ConnectivityDetector.ConnectionState.VALIDATED);
@@ -132,7 +139,11 @@ public class OfflineIndicatorController implements ConnectivityDetector.Observer
     private void updateOfflineIndicator(boolean isOnline) {
         if (isOnline != mIsOnline) {
             if (isOnline) {
+                mWasOnlineForRequiredDuration = false;
                 mLastOnlineTime = SystemClock.elapsedRealtime();
+            } else {
+                mWasOnlineForRequiredDuration = SystemClock.elapsedRealtime() - mLastOnlineTime
+                        >= getTimeToWaitForStableOffline();
             }
             mIsOnline = isOnline;
         }
@@ -224,9 +235,7 @@ public class OfflineIndicatorController implements ConnectivityDetector.Observer
         // be shown if the user has been continuously online for the required duration, then goes
         // back to being offline.
         // TODO(jianli): keep these values in shared prefernces. (http://crbug.com/879725)
-        if (mHasOfflineIndicatorShownSinceActivityResumed
-                && SystemClock.elapsedRealtime() - mLastOnlineTime
-                        < getTimeToWaitForStableOffline()) {
+        if (mHasOfflineIndicatorShownSinceActivityResumed && !mWasOnlineForRequiredDuration) {
             return;
         }
 
@@ -257,7 +266,8 @@ public class OfflineIndicatorController implements ConnectivityDetector.Observer
         mHasOfflineIndicatorShownSinceActivityResumed = true;
     }
 
-    private void hideOfflineIndicator(ChromeActivity chromeActivity) {
+    @VisibleForTesting
+    void hideOfflineIndicator(ChromeActivity chromeActivity) {
         if (!mIsShowingOfflineIndicator) return;
 
         if (isUsingTopSnackbar()) {

chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/indicator/OfflineIndicatorControllerTest.java

@@ -14,6 +14,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.ApplicationState;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.chrome.R;
@@ -270,9 +271,66 @@ public class OfflineIndicatorControllerTest {
         checkOfflineIndicatorVisibility(mActivityTestRule.getActivity(), true);
     }
 
+    @Test
+    @MediumTest
+    public void testReshowOfflineIndicatorWhenResumed() throws Exception {
+        EmbeddedTestServer testServer =
+                EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
+        String testUrl = testServer.getURL(TEST_PAGE);
+
+        // Load a page.
+        loadPage(testUrl);
+
+        // Disconnect the network.
+        setNetworkConnectivity(false);
+
+        // Offline indicator should be shown.
+        checkOfflineIndicatorVisibility(mActivityTestRule.getActivity(), true);
+
+        // Hide offline indicator.
+        hideOfflineIndicator(mActivityTestRule.getActivity());
+
+        // Simulate switching to other app and then coming back.
+        setApplicationState(ApplicationState.HAS_STOPPED_ACTIVITIES);
+        setApplicationState(ApplicationState.HAS_RUNNING_ACTIVITIES);
+
+        // Offline indicator should be shown again.
+        checkOfflineIndicatorVisibility(mActivityTestRule.getActivity(), true);
+    }
+
+    @Test
+    @MediumTest
+    public void testDoNotShowOfflineIndicatorWhenTemporarilyPaused() throws Exception {
+        EmbeddedTestServer testServer =
+                EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
+        String testUrl = testServer.getURL(TEST_PAGE);
+
+        // Load a page.
+        loadPage(testUrl);
+
+        // Disconnect the network.
+        setNetworkConnectivity(false);
+
+        // Offline indicator should be shown.
+        checkOfflineIndicatorVisibility(mActivityTestRule.getActivity(), true);
+
+        // Hide offline indicator.
+        hideOfflineIndicator(mActivityTestRule.getActivity());
+
+        // The paused state can be set when the activity is temporarily covered by another
+        // activity's Fragment. So switching to this state temporarily should not bring back
+        // the offline indicator.
+        setApplicationState(ApplicationState.HAS_PAUSED_ACTIVITIES);
+        setApplicationState(ApplicationState.HAS_RUNNING_ACTIVITIES);
+
+        // Offline indicator should not be shown.
+        checkOfflineIndicatorVisibility(mActivityTestRule.getActivity(), false);
+    }
+
     private void setNetworkConnectivity(boolean connected) {
         mIsConnected = connected;
         ThreadUtils.runOnUiThreadBlocking(() -> {
+            NetworkChangeNotifier.forceConnectivityState(connected);
             OfflineIndicatorController.getInstance()
                     .getConnectivityDetectorForTesting()
                     .setConnectionState(connected
@@ -281,6 +339,12 @@ public class OfflineIndicatorControllerTest {
         });
     }
 
+    private void setApplicationState(int newState) {
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            OfflineIndicatorController.getInstance().onApplicationStateChange(newState);
+        });
+    }
+
     private void loadPage(String pageUrl) throws Exception {
         Tab tab = mActivityTestRule.getActivity().getActivityTab();
 
@@ -363,6 +427,15 @@ public class OfflineIndicatorControllerTest {
                 });
     }
 
+    private static void hideOfflineIndicator(ChromeActivity activity) {
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                OfflineIndicatorController.getInstance().hideOfflineIndicator(activity);
+            }
+        });
+    }
+
     private static boolean isErrorPage(final Tab tab) {
         final boolean[] isShowingError = new boolean[1];
         ThreadUtils.runOnUiThreadBlocking(() -> { isShowingError[0] = tab.isShowingErrorPage(); });