Skip to content

Commit

Permalink
[External intents] Use delegate's Context(Wrapper) rather than Activity
Browse files Browse the repository at this point in the history
I will shortly be moving //chrome's implementation of warning the user
on launching an intent incognito mode into ExternalNavigationHandler
itself for reuse by WebLayer. However, there is a wrinkle that I need
to solve in order to do so: in WebLayer, the string resources that are
fetched from the Context in order to build the alert dialog are only
available from the delegate impl's ContextWrapper that wraps the
Activity, not from the Activity itself. This is because that
ContextWrapper implements special logic to fetch the resources via the
Context of the embedder.

To solve this wrinkle and facilitate the upcoming componentization, this
CL does the following:

1. Changes ExternalNavigationDelegate#getActivityContext() to
   ExternalNavigationDelegate#getContext().
2. Changes the two ExternalNavigationHandler sites that interact with
   this method via ExternalNavigationHandler#getAvailableContext() to
   explicitly handle logic for the case where the delegate's Activity is
   available/not available explicitly.
3. Moves ExternalNavigationHandler#getAvailableContext() into //chrome's
   ExternalNavigationDelegateImpl, as it is now only used in //chrome.
4. Relaxes //chrome's
   ExternalNavigationDelegateImpl#startIncognitoIntentInternal() to
   check that its Context *wraps* an Activity rather than *is* an
   Activity in preparation for componentizing this method and invoking
   it via the WebLayer embedder, where the Context returned from
   ExternalNavigationDelegate#getContext() will indeed wrap rather than
   be an Activity. Note that the creation of an alert dialog does not
   require that the passed-in Context *be* an Activity. I investigated
   where the original check came from, and it was from a CL that was
   eliminating ExternalNavigationDelegateImpl.java caching an Activity
   field in an ivar as part of supporting tab reparenting
   (https://codereview.chromium.org/1636573004/). In that context, the
   check appears to be simply verifying whether the Tab is actually
   associated with an Activity or not. The current check preserves the
   conceptual verification.

Bug: 1063399
Change-Id: I3de91f43e17d4691a8a67f81bb5091280ae1fc3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2283604
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#787182}
  • Loading branch information
colinblundell authored and Commit Bot committed Jul 10, 2020
1 parent 2429fa4 commit dc02d32
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,21 @@ public void onDestroyed(Tab tab) {
}

@Override
public Activity getActivityContext() {
public Context getContext() {
if (mTab.getWindowAndroid() == null) return null;
return ContextUtils.activityFromContext(mTab.getWindowAndroid().getContext().get());
return mTab.getWindowAndroid().getContext().get();
}

/**
* Gets the {@link Activity} linked to this instance if it is available. At times this object
* might not have an associated Activity, in which case the ApplicationContext is returned.
* @return The activity {@link Context} if it can be reached.
* Application {@link Context} if not.
*/
protected final Context getAvailableContext() {
return ExternalNavigationHandler.getAvailableContext(this);
Activity activityContext = ContextUtils.activityFromContext(getContext());
if (activityContext == null) return ContextUtils.getApplicationContext();
return activityContext;
}

/**
Expand Down Expand Up @@ -175,10 +183,9 @@ private boolean startIncognitoIntentInternal(final Intent intent, final String r
final String fallbackUrl, final boolean needsToCloseTab, final boolean proxy) {
if (!hasValidTab()) return false;
Context context = mTab.getWindowAndroid().getContext().get();
if (!(context instanceof Activity)) return false;
if (ContextUtils.activityFromContext(context) == null) return false;

Activity activity = (Activity) context;
new UiUtils.CompatibleAlertDialogBuilder(activity, R.style.Theme_Chromium_AlertDialog)
new UiUtils.CompatibleAlertDialogBuilder(context, R.style.Theme_Chromium_AlertDialog)
.setTitle(R.string.external_app_leave_incognito_warning_title)
.setMessage(R.string.external_app_leave_incognito_warning)
.setPositiveButton(R.string.external_app_leave_incognito_leave,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

package org.chromium.components.external_intents;

import android.app.Activity;
import android.content.Context;
import android.content.Intent;

import androidx.annotation.IntDef;
Expand All @@ -25,10 +25,15 @@
*/
public interface ExternalNavigationDelegate {
/**
* Returns the Activity with which this delegate is associated, or null if there
* is no such Activity at the time of invocation.
* Returns the Context with which this delegate is associated, or null if there is no such
* Context at the time of invocation. The returned Context may or may not be a wrapper around an
* Activity with which the delegate is associated. Note that when obtaining resources, however,
* the handler should do so directly via the returned Context (i.e., not via the Activity that
* it is wrapping, even if it is in fact wrapping one). The reason is that some embedders handle
* resource fetching via special logic in the ContextWrapper object that is wrapping the
* Activity.
*/
Activity getActivityContext();
Context getContext();

/**
* Determine if this app is the default or only handler for a given intent. If true, this app
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1394,19 +1394,6 @@ private static void forcePdfViewerAsIntentHandlerIfNeeded(Intent intent) {
resolveIntent(intent, true /* allowSelfOpen (ignored) */);
}

/**
* Get a {@link Context} linked to this instance with preference to the delegate's {@link
* Activity}. At times the delegate might not have an associated Activity, in which case the
* ApplicationContext is returned.
* @return The activity {@link Context} if it can be reached.
* Application {@link Context} if not.
*/
public static Context getAvailableContext(ExternalNavigationDelegate delegate) {
Activity activityContext = delegate.getActivityContext();
if (activityContext == null) return ContextUtils.getApplicationContext();
return activityContext;
}

/**
* Retrieve the best activity for the given intent. If a default activity is provided,
* choose the default one. Otherwise, return the Intent picker if there are more than one
Expand Down Expand Up @@ -1467,8 +1454,13 @@ public static void startActivity(
if (proxy) {
delegate.dispatchAuthenticatedIntent(intent);
} else {
Context context = getAvailableContext(delegate);
if (!(context instanceof Activity)) intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
// Start the activity via the current activity if possible, and otherwise as a new
// task from the application context.
Context context = ContextUtils.activityFromContext(delegate.getContext());
if (context == null) {
context = ContextUtils.getApplicationContext();
intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
}
context.startActivity(intent);
}
recordExternalNavigationDispatched(intent);
Expand Down Expand Up @@ -1519,9 +1511,9 @@ private boolean startActivityIfNeededInternal(Intent intent, boolean proxy) {
mDelegate.dispatchAuthenticatedIntent(intent);
activityWasLaunched = true;
} else {
Context context = getAvailableContext(mDelegate);
if (context instanceof Activity) {
activityWasLaunched = ((Activity) context).startActivityIfNeeded(intent, -1);
Activity activity = ContextUtils.activityFromContext(mDelegate.getContext());
if (activity != null) {
activityWasLaunched = activity.startActivityIfNeeded(intent, -1);
} else {
activityWasLaunched = false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package org.chromium.components.external_intents;

import android.annotation.SuppressLint;
import android.app.Activity;
import android.content.Context;
import android.content.ContextWrapper;
import android.content.Intent;
Expand Down Expand Up @@ -2054,7 +2053,7 @@ public List<ResolveInfo> queryIntentActivities(Intent intent) {
}

@Override
public Activity getActivityContext() {
public Context getContext() {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

package org.chromium.weblayer_private;

import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.content.pm.ResolveInfo;

Expand Down Expand Up @@ -43,8 +43,8 @@ public void onTabDestroyed() {
}

@Override
public Activity getActivityContext() {
return ContextUtils.activityFromContext(mTab.getBrowser().getContext());
public Context getContext() {
return mTab.getBrowser().getContext();
}

@Override
Expand Down

0 comments on commit dc02d32

Please sign in to comment.