[Offline indicator v2] Attempt to fix NPE

There is a NullPointerException in OfflineIndicatorControllerV2 that
seem to be caused by a posted
ConnectivityDetector#performConnectivityCheck().

While we can't repro the crash, this CL adds a call to
from the Handler. It also removes any posted runnables in
OfflineIndicatorControllerV2#destroy() that may otherwise cause similar
issues.

Bug: 1085567
Change-Id: I548dbe4b0c54d382cd9e5684f5e252565f9fc337
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216388
Commit-Queue: Sinan Sahin <sinansahin@google.com>
Reviewed-by: Jian Li <jianli@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772392}

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

@@ -14,6 +14,7 @@ import android.os.Handler;
 import android.os.SystemClock;
 
 import androidx.annotation.IntDef;
+import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.Callback;
@@ -202,6 +203,8 @@ public class ConnectivityDetector implements NetworkChangeNotifier.ConnectionTyp
     private static String sProbeMethod = PROBE_METHOD;
     private static int sConnectivityCheckInitialDelayMs = CONNECTIVITY_CHECK_INITIAL_DELAY_MS;
 
+    /** |mObserver| will be null after destruction. */
+    @Nullable
     private Observer mObserver;
     private Delegate mDelegate;
 
@@ -230,6 +233,8 @@ public class ConnectivityDetector implements NetworkChangeNotifier.ConnectionTyp
 
     public void destroy() {
         NetworkChangeNotifier.removeConnectionTypeObserver(this);
+        stopConnectivityCheck();
+        mObserver = null;
     }
 
     public void detect() {
@@ -481,7 +486,7 @@ public class ConnectivityDetector implements NetworkChangeNotifier.ConnectionTyp
     void setConnectionState(@ConnectionState int connectionState) {
         if (mConnectionState == connectionState) return;
         mConnectionState = connectionState;
-        mObserver.onConnectionStateChanged(mConnectionState);
+        if (mObserver != null) mObserver.onConnectionStateChanged(mConnectionState);
     }
 
     @VisibleForTesting

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

@@ -60,6 +60,7 @@ public class OfflineIndicatorControllerV2 implements ConnectivityDetector.Observ
     private Supplier<Boolean> mCanAnimateBrowserControlsSupplier;
     private Callback<Boolean> mOnUrlBarFocusChanged;
     private Runnable mShowRunnable;
+    private Runnable mUpdateAndHideRunnable;
     private Runnable mHideRunnable;
     private Runnable mOnUrlBarUnfocusedRunnable;
     private Runnable mUpdateStatusIndicatorDelayedRunnable;
@@ -109,6 +110,11 @@ public class OfflineIndicatorControllerV2 implements ConnectivityDetector.Observ
         };
 
         mHideRunnable = () -> {
+            mHandler.postDelayed(
+                    () -> mStatusIndicator.hide(), STATUS_INDICATOR_WAIT_BEFORE_HIDE_DURATION_MS);
+        };
+
+        mUpdateAndHideRunnable = () -> {
             RecordUserAction.record("OfflineIndicator.Hidden");
             final long shownDuration =
                     TimeUnit.MICROSECONDS.toMillis(TimeUtilsJni.get().getTimeTicksNowUs())
@@ -125,13 +131,9 @@ public class OfflineIndicatorControllerV2 implements ConnectivityDetector.Observ
             final Drawable statusIcon = mContext.getDrawable(R.drawable.ic_globe_24dp);
             final int iconTint = ApiCompatibilityUtils.getColor(
                     mContext.getResources(), R.color.default_icon_color_inverse);
-            Runnable hide = () -> {
-                mHandler.postDelayed(() -> mStatusIndicator.hide(),
-                        STATUS_INDICATOR_WAIT_BEFORE_HIDE_DURATION_MS);
-            };
             mStatusIndicator.updateContent(
                     mContext.getString(R.string.offline_indicator_v2_back_online_text), statusIcon,
-                    backgroundColor, textColor, iconTint, hide);
+                    backgroundColor, textColor, iconTint, mHideRunnable);
         };
 
         mIsUrlBarFocusedSupplier = isUrlBarFocusedSupplier;
@@ -192,6 +194,9 @@ public class OfflineIndicatorControllerV2 implements ConnectivityDetector.Observ
         }
 
         mOnUrlBarFocusChanged = null;
+
+        mHandler.removeCallbacks(mHideRunnable);
+        mHandler.removeCallbacks(mUpdateStatusIndicatorDelayedRunnable);
     }
 
     private void updateStatusIndicator(boolean offline) {
@@ -202,17 +207,17 @@ public class OfflineIndicatorControllerV2 implements ConnectivityDetector.Observ
             // runnable. E.g, without this condition, we would be trying to hide the indicator when
             // it's not shown if we were set to show the widget but then went back online.
             if ((!offline && mOnUrlBarUnfocusedRunnable == mShowRunnable)
-                    || (offline && mOnUrlBarUnfocusedRunnable == mHideRunnable)) {
+                    || (offline && mOnUrlBarUnfocusedRunnable == mUpdateAndHideRunnable)) {
                 mOnUrlBarUnfocusedRunnable = null;
                 return;
             }
-            mOnUrlBarUnfocusedRunnable = offline ? mShowRunnable : mHideRunnable;
+            mOnUrlBarUnfocusedRunnable = offline ? mShowRunnable : mUpdateAndHideRunnable;
             surfaceState = mCanAnimateBrowserControlsSupplier.get()
                     ? UmaEnum.CAN_ANIMATE_NATIVE_CONTROLS_OMNIBOX_FOCUSED
                     : UmaEnum.CANNOT_ANIMATE_NATIVE_CONTROLS_OMNIBOX_FOCUSED;
         } else {
             assert mOnUrlBarUnfocusedRunnable == null;
-            (offline ? mShowRunnable : mHideRunnable).run();
+            (offline ? mShowRunnable : mUpdateAndHideRunnable).run();
             surfaceState = mCanAnimateBrowserControlsSupplier.get()
                     ? UmaEnum.CAN_ANIMATE_NATIVE_CONTROLS
                     : UmaEnum.CANNOT_ANIMATE_NATIVE_CONTROLS;