Allow deferred intent when the screen is locked

chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser;
 
 import android.app.KeyguardManager;
 import android.app.PendingIntent;
 import android.app.SearchManager;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
+import android.content.IntentFilter;
 import android.net.Uri;
 import android.os.Bundle;
 import android.os.SystemClock;
 import android.provider.Browser;
 import android.provider.MediaStore;
 import android.speech.RecognizerResultsIntent;
 import android.text.TextUtils;
 import android.util.Pair;
 
 import org.chromium.base.ApiCompatibilityUtils;
@@ skipping 196 matching lines @@
     //                publicly available.
     private static final String TAB_ACTIVITY_COMPONENT_CLASS_NAME =
             "com.google.android.apps.chrome.Main";
 
     private static boolean sTestIntentsEnabled;
 
     private final IntentHandlerDelegate mDelegate;
     private final String mPackageName;
     private KeyguardManager mKeyguardManager;
 
+    /**
+     * Receiver for screen unlock broadcast.
+     */
+    private KeyguardBroadcastReceiver mKeyguardBroadcastReceiver;
+
     public static enum TabOpenType {
         OPEN_NEW_TAB,
         // Tab is reused only if the URLs perfectly match.
         REUSE_URL_MATCHING_TAB_ELSE_NEW_TAB,
         // Tab is reused only if there's an existing tab opened by the same app ID.
         REUSE_APP_ID_MATCHING_TAB_ELSE_NEW_TAB,
         CLOBBER_CURRENT_TAB,
         BRING_TAB_TO_FRONT,
         // Opens a new incognito tab.
         OPEN_NEW_INCOGNITO_TAB,
@@ skipping 85 matching lines @@
         List<String> packages = IntentUtils.safeGetStringArrayListExtra(intent,
                 IntentHandler.EXTRA_EXTERNAL_NAV_PACKAGES);
         if (packages != null && packages.size() > 0) {
             RecordUserAction.record("MobileExternalNavigationReceived");
             for (String name : packages) {
                 RapporServiceBridge.sampleString("Android.ExternalNavigationNotChosen", name);
             }
         }
     }
 
+    @VisibleForTesting
+    void tryUnregisterLockscreenIntentReceiver(Context context, Intent intent) {
+        // Only unregister receiver when the intent was specially marked or the receiver no longer
+        // contain any valid deferred intent.
+        // If the intent does not contain the special mark, it should be treated as normal intent
+        // instead. This is helping the scenario when the screen is unlocked, another intent is
+        // fired before the broadcast.

[Maria, 2016/12/13 21:36:09]

I think that if another event was fired before the broadcast, but after unlock,
we should unregister receiver and drop the deferred intent..

[Sherry, 2016/12/13 22:13:54]

If the deferred intent is still within the lifetime range, I believe it is
better to wait for the deferred intent since it might be triggered very soon.
Also, the lifetime for the deferred intent is not too long, so it won't hold the
receiver forever. The worst case would be after the deferred intent lifetime
expired, the broadcast is not triggered, and it will be recycled by next coming
intent. 

[Sherry, 2016/12/13 22:32:29]

My bad, you are right. If the new irrelevant intent was fired before the
deferred intent. We should drop the deferred intent and unregister the receiver.
The current logic also handle the intents this way. I will update the comment
here.

[Ted C, 2016/12/13 22:47:13]

I agree with Maria.  If another intent was received, we should cancel the
deferred one.  A new intent received implies the user took a different action. 
If we chose to honor the deferred intent after the new user driven intent, it
could potentially clobber their action or at least impede it.

The likelihood of it happening is very low, but I still don't think acting on it
makes sense.

Ha...just saw your update...looks like we are on the same page.

+        if (mKeyguardBroadcastReceiver != null
+                && !mKeyguardBroadcastReceiver.hasValidDeferredIntent(intent)) {
+            context.unregisterReceiver(mKeyguardBroadcastReceiver);
+            mKeyguardBroadcastReceiver = null;
+        }
+    }
+
     /**
      * Handles an Intent after the ChromeTabbedActivity decides that it shouldn't ignore the
      * Intent.
-     *
+     * @param context Android Context.
+     * @param intent Target intent.
      * @return Whether the Intent was successfully handled.
      */
     boolean onNewIntent(Context context, Intent intent) {
+        tryUnregisterLockscreenIntentReceiver(context, intent);
+
         assert intentHasValidUrl(intent);
         String url = getUrlFromIntent(intent);
         boolean hasUserGesture =
                 IntentWithGesturesHandler.getInstance().getUserGestureAndClear(intent);
         TabOpenType tabOpenType = getTabOpenType(intent);
         int tabIdToBringToFront = IntentUtils.safeGetIntExtra(
                 intent, TabOpenType.BRING_TAB_TO_FRONT.name(), Tab.INVALID_TAB_ID);
         if (url == null && tabIdToBringToFront == Tab.INVALID_TAB_ID
                 && tabOpenType != TabOpenType.OPEN_NEW_INCOGNITO_TAB) {
             return handleWebSearchIntent(intent);
@@ skipping 330 matching lines @@
                     }
 
                     Log.w(TAG, "Ignoring internal Chrome URL from untrustworthy source.");
                     return true;
                 }
             }
 
             // We must check for screen state at this point.
             // These might be slow.
             boolean internalOrVisible = isInternal || isIntentUserVisible(context);
-            return !internalOrVisible;
+            if (!internalOrVisible) {
+                // Register keyguard broadcast receiver to receive ACTION_USER_PRESENT when the
+                // screen is unlocked.
+                // Avoid register KeyguardBroadcastReceiver if there is already one cached to allow
+                // unregister on the same instance.
+                // The ACTION_USER_PRESENT is sent by platform to indicates when user is present.
+                if (mKeyguardBroadcastReceiver == null) {
+                    mKeyguardBroadcastReceiver = new KeyguardBroadcastReceiver(intent);
+                    context.registerReceiver(mKeyguardBroadcastReceiver,
+                            new IntentFilter(Intent.ACTION_USER_PRESENT));
+                } else {
+                    mKeyguardBroadcastReceiver.updateDeferredIntent(intent);
+                }

[Maria, 2016/12/13 21:36:09]

maybe move these lines into a method registerLockscreenIntentReceiver to stay
symmetric with unregistering.

[Sherry, 2016/12/13 22:13:54]

Sounds good to me. Will update.

[Ted C, 2016/12/13 22:47:13]

Also, I would have the receiver unregister itself after
VALID_DEFERRED_PERIOD_MS.

If we get an intent while the screen is locked, but it never becomes unlocked,
then I think we should proactively remove the receiver.

[Sherry, 2016/12/14 00:54:07]

Then we need a background task that continuously check the time and drop the
intent on expiration. This is less efficient than holding the instance until
some intent to trigger the invalidation.

+                return true;
+            }
+            return false;
         } catch (Throwable t) {
             return true;
         }
     }
 
     @VisibleForTesting
+    void setKeyguardBroadcastReceiverForTesting(KeyguardBroadcastReceiver receiver) {
+        mKeyguardBroadcastReceiver = receiver;
+    }
+
+    @VisibleForTesting
     boolean intentHasValidUrl(Intent intent) {
         String url = getUrlFromIntent(intent);
 
         // Always drop insecure urls.
         if (url != null && isJavascriptSchemeOrInvalidUrl(url)) {
             return false;
         }
         return true;
     }
 
@@ skipping 46 matching lines @@
         if (isChromeToken(token, context)) {
             return true;
         }
         if (ExternalAuthUtils.getInstance().isGoogleSigned(
                     context, ApiCompatibilityUtils.getCreatorPackage(token))) {
             return true;
         }
         return false;
     }
 
-    private boolean isIntentUserVisible(Context context) {
+    @VisibleForTesting
+    boolean isIntentUserVisible(Context context) {
         // Only process Intents if the screen is on and the device is unlocked;
         // i.e. the user will see what is going on.
         if (mKeyguardManager == null) {
             mKeyguardManager = (KeyguardManager) context.getSystemService(Context.KEYGUARD_SERVICE);
         }
         if (!ApiCompatibilityUtils.isInteractive(context)) return false;
         return !ApiCompatibilityUtils.isDeviceProvisioned(context)
                 || !mKeyguardManager.inKeyguardRestrictedInputMode();
     }
 
@@ skipping 203 matching lines @@
         if (transitionType == PageTransition.TYPED) {
             return transitionType;
         } else if (transitionType != PageTransition.LINK
                 && isIntentChromeOrFirstParty(intent, context)) {
             // 1st party applications may specify any transition type.
             return transitionType;
         }
         return defaultTransition;
     }
 }