Commit 6b18ee1c authored by Jinsuk Kim's avatar Jinsuk Kim Committed by Commit Bot

Android: Avoid calling TabImpl.getActivity() where possible

For some of the callsites of TabImpl.getActivity(), what they
actually need is not ChromeActivity, but either Context or
Activity. This CL replaces them with Tab.getContext().

TabImpl.getActivity() and Tab.getContext() is different in that
the former returns null if the tab is not attached to
ChromeActivity (or not attached at all) while the latter returns
the application context for such case.

The callsites updated in this CL are not affected by the
difference due to one of following reasons:

- a null check to see if the tab is detached is already in
  place (ModuleInstallUi)

- it is okay to proceed with the application
  context (ContextualSearchTabHelper - works for ChromeActivity
  only. For non-ChromeActivity, its scaleFactor could be set
  wrongly but CS won't be activated anyway)

- getActivity() is always returning a non-null activity
  i.e. Tab.getContext() returns activity
  contenxt (LayerTitleCache, ExploreSitesPage, SurveyInfoBar,
  PictureInPictureActivity, TabParentIntent, ArConteInstallUtils)

- a null check was added newly to check if the tab is in the
  detached state (AddToHomescreenInstaller - it is used by
  ChromeActivity-inherited activity only, therefore
  TabImpl.getActivity() returning null due to the tab being
  attached to non-ChromeActivity won't happen).

Bug: 995903
Change-Id: I763787c57db62e3af61ee7f88bf172b1b93c45ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2109392
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751977}
parent 9880e60b
...@@ -1591,6 +1591,7 @@ chrome_java_sources = [ ...@@ -1591,6 +1591,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/tab/TabStateBrowserControlsVisibilityDelegate.java", "java/src/org/chromium/chrome/browser/tab/TabStateBrowserControlsVisibilityDelegate.java",
"java/src/org/chromium/chrome/browser/tab/TabThemeColorHelper.java", "java/src/org/chromium/chrome/browser/tab/TabThemeColorHelper.java",
"java/src/org/chromium/chrome/browser/tab/TabUma.java", "java/src/org/chromium/chrome/browser/tab/TabUma.java",
"java/src/org/chromium/chrome/browser/tab/TabUtils.java",
"java/src/org/chromium/chrome/browser/tab/TabViewAndroidDelegate.java", "java/src/org/chromium/chrome/browser/tab/TabViewAndroidDelegate.java",
"java/src/org/chromium/chrome/browser/tab/TabWebContentsDelegateAndroid.java", "java/src/org/chromium/chrome/browser/tab/TabWebContentsDelegateAndroid.java",
"java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java", "java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java",
......
...@@ -19,7 +19,6 @@ import org.chromium.chrome.browser.flags.ChromeFeatureList; ...@@ -19,7 +19,6 @@ import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabFavicon; import org.chromium.chrome.browser.tab.TabFavicon;
import org.chromium.chrome.browser.tab.TabImpl;
import org.chromium.chrome.browser.tabmodel.TabModelSelector; import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.ui.favicon.FaviconHelper; import org.chromium.chrome.browser.ui.favicon.FaviconHelper;
import org.chromium.chrome.browser.ui.favicon.FaviconHelper.DefaultFaviconHelper; import org.chromium.chrome.browser.ui.favicon.FaviconHelper.DefaultFaviconHelper;
...@@ -127,8 +126,7 @@ public class LayerTitleCache implements TitleCache { ...@@ -127,8 +126,7 @@ public class LayerTitleCache implements TitleCache {
private String getUpdatedTitleInternal(Tab tab, String titleString, private String getUpdatedTitleInternal(Tab tab, String titleString,
boolean fetchFaviconFromHistory) { boolean fetchFaviconFromHistory) {
final int tabId = tab.getId(); final int tabId = tab.getId();
boolean isHTSEnabled = boolean isHTSEnabled = !DeviceFormFactor.isNonMultiDisplayContextOnTablet(tab.getContext())
!DeviceFormFactor.isNonMultiDisplayContextOnTablet(((TabImpl) tab).getActivity())
&& ChromeFeatureList.isEnabled(ChromeFeatureList.HORIZONTAL_TAB_SWITCHER_ANDROID); && ChromeFeatureList.isEnabled(ChromeFeatureList.HORIZONTAL_TAB_SWITCHER_ANDROID);
boolean isDarkTheme = tab.isIncognito() && !isHTSEnabled; boolean isDarkTheme = tab.isIncognito() && !isHTSEnabled;
Bitmap originalFavicon = TabFavicon.getBitmap(tab); Bitmap originalFavicon = TabFavicon.getBitmap(tab);
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
package org.chromium.chrome.browser.contextualsearch; package org.chromium.chrome.browser.contextualsearch;
import android.app.Activity; import android.app.Activity;
import android.content.Context;
import android.view.ContextMenu; import android.view.ContextMenu;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
...@@ -97,10 +98,8 @@ public class ContextualSearchTabHelper ...@@ -97,10 +98,8 @@ public class ContextualSearchTabHelper
NetworkChangeNotifier.addConnectionTypeObserver(this); NetworkChangeNotifier.addConnectionTypeObserver(this);
} }
float scaleFactor = 1.f; float scaleFactor = 1.f;
if (tab != null && ((TabImpl) tab).getActivity() != null Context context = tab != null ? tab.getContext() : null;
&& ((TabImpl) tab).getActivity().getResources() != null) { if (context != null) scaleFactor /= context.getResources().getDisplayMetrics().density;
scaleFactor /= ((TabImpl) tab).getActivity().getResources().getDisplayMetrics().density;
}
mPxToDp = scaleFactor; mPxToDp = scaleFactor;
} }
......
...@@ -235,8 +235,8 @@ public class ExploreSitesPage extends BasicNativePage { ...@@ -235,8 +235,8 @@ public class ExploreSitesPage extends BasicNativePage {
NativePageNavigationDelegateImpl navDelegate = new NativePageNavigationDelegateImpl( NativePageNavigationDelegateImpl navDelegate = new NativePageNavigationDelegateImpl(
activity, mProfile, host, activity.getTabModelSelector(), mTab); activity, mProfile, host, activity.getTabModelSelector(), mTab);
// Don't direct reference activity because it might change if tab is reparented. // ExploreSitePage is recreated upon reparenting. Safe to use |activity|.
Runnable closeContextMenuCallback = () -> ((TabImpl) mTab).getActivity().closeContextMenu(); Runnable closeContextMenuCallback = activity::closeContextMenu;
mContextMenuManager = createContextMenuManager( mContextMenuManager = createContextMenuManager(
navDelegate, closeContextMenuCallback, CONTEXT_MENU_USER_ACTION_PREFIX); navDelegate, closeContextMenuCallback, CONTEXT_MENU_USER_ACTION_PREFIX);
......
...@@ -18,7 +18,7 @@ import org.chromium.chrome.browser.survey.SurveyController; ...@@ -18,7 +18,7 @@ import org.chromium.chrome.browser.survey.SurveyController;
import org.chromium.chrome.browser.tab.EmptyTabObserver; import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabHidingType; import org.chromium.chrome.browser.tab.TabHidingType;
import org.chromium.chrome.browser.tab.TabImpl; import org.chromium.chrome.browser.tab.TabUtils;
import org.chromium.chrome.browser.ui.messages.infobar.InfoBarCompactLayout; import org.chromium.chrome.browser.ui.messages.infobar.InfoBarCompactLayout;
import org.chromium.chrome.browser.util.AccessibilityUtil; import org.chromium.chrome.browser.util.AccessibilityUtil;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
...@@ -177,7 +177,7 @@ public class SurveyInfoBar extends InfoBar { ...@@ -177,7 +177,7 @@ public class SurveyInfoBar extends InfoBar {
mDelegate.onSurveyTriggered(); mDelegate.onSurveyTriggered();
SurveyController.getInstance().showSurveyIfAvailable( SurveyController.getInstance().showSurveyIfAvailable(
((TabImpl) tab).getActivity(), mSiteId, mShowAsBottomSheet, mDisplayLogoResId); TabUtils.getActivity(tab), mSiteId, mShowAsBottomSheet, mDisplayLogoResId);
super.onCloseButtonClicked(); super.onCloseButtonClicked();
} }
......
...@@ -28,7 +28,7 @@ import org.chromium.chrome.R; ...@@ -28,7 +28,7 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.init.AsyncInitializationActivity; import org.chromium.chrome.browser.init.AsyncInitializationActivity;
import org.chromium.chrome.browser.tab.EmptyTabObserver; import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabImpl; import org.chromium.chrome.browser.tab.TabUtils;
import org.chromium.chrome.browser.thinwebview.CompositorView; import org.chromium.chrome.browser.thinwebview.CompositorView;
import org.chromium.chrome.browser.thinwebview.CompositorViewFactory; import org.chromium.chrome.browser.thinwebview.CompositorViewFactory;
import org.chromium.chrome.browser.thinwebview.ThinWebViewConstraints; import org.chromium.chrome.browser.thinwebview.ThinWebViewConstraints;
...@@ -265,7 +265,7 @@ public class PictureInPictureActivity extends AsyncInitializationActivity { ...@@ -265,7 +265,7 @@ public class PictureInPictureActivity extends AsyncInitializationActivity {
sNativeOverlayWindowAndroid = nativeOverlayWindowAndroid; sNativeOverlayWindowAndroid = nativeOverlayWindowAndroid;
sInitiatorTab = (Tab) initiatorTab; sInitiatorTab = (Tab) initiatorTab;
sInitiatorTabTaskID = ((TabImpl) sInitiatorTab).getActivity().getTaskId(); sInitiatorTabTaskID = TabUtils.getActivity(sInitiatorTab).getTaskId();
sTabObserver = new InitiatorTabObserver(); sTabObserver = new InitiatorTabObserver();
sInitiatorTab.addObserver(sTabObserver); sInitiatorTab.addObserver(sTabObserver);
......
...@@ -10,7 +10,7 @@ import org.chromium.chrome.R; ...@@ -10,7 +10,7 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.infobar.InfoBarIdentifier; import org.chromium.chrome.browser.infobar.InfoBarIdentifier;
import org.chromium.chrome.browser.infobar.SimpleConfirmInfoBarBuilder; import org.chromium.chrome.browser.infobar.SimpleConfirmInfoBarBuilder;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabImpl; import org.chromium.chrome.browser.tab.TabUtils;
import org.chromium.ui.widget.Toast; import org.chromium.ui.widget.Toast;
/** /**
...@@ -48,7 +48,7 @@ public class ModuleInstallUi { ...@@ -48,7 +48,7 @@ public class ModuleInstallUi {
/** Show UI indicating the start of a module install. */ /** Show UI indicating the start of a module install. */
public void showInstallStartUi() { public void showInstallStartUi() {
Context context = ((TabImpl) mTab).getActivity(); Context context = TabUtils.getActivity(mTab);
if (context == null) { if (context == null) {
// Tab is detached. Don't show UI. // Tab is detached. Don't show UI.
return; return;
...@@ -67,7 +67,7 @@ public class ModuleInstallUi { ...@@ -67,7 +67,7 @@ public class ModuleInstallUi {
mInstallStartToast = null; mInstallStartToast = null;
} }
Context context = ((TabImpl) mTab).getActivity(); Context context = TabUtils.getActivity(mTab);
if (context == null) { if (context == null) {
// Tab is detached. Don't show UI. // Tab is detached. Don't show UI.
return; return;
...@@ -85,7 +85,7 @@ public class ModuleInstallUi { ...@@ -85,7 +85,7 @@ public class ModuleInstallUi {
mInstallStartToast = null; mInstallStartToast = null;
} }
Context context = ((TabImpl) mTab).getActivity(); Context context = TabUtils.getActivity(mTab);
if (context == null) { if (context == null) {
// Tab is detached. Cancel. // Tab is detached. Cancel.
if (mFailureUiListener != null) mFailureUiListener.onFailureUiResponse(false); if (mFailureUiListener != null) mFailureUiListener.onFailureUiResponse(false);
......
...@@ -32,6 +32,7 @@ import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate; ...@@ -32,6 +32,7 @@ import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate;
import org.chromium.chrome.browser.suggestions.tile.TileGroup; import org.chromium.chrome.browser.suggestions.tile.TileGroup;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabImpl; import org.chromium.chrome.browser.tab.TabImpl;
import org.chromium.chrome.browser.tab.TabUtils;
import org.chromium.components.browser_ui.widget.displaystyle.UiConfig; import org.chromium.components.browser_ui.widget.displaystyle.UiConfig;
import org.chromium.components.browser_ui.widget.displaystyle.ViewResizer; import org.chromium.components.browser_ui.widget.displaystyle.ViewResizer;
import org.chromium.ui.base.ViewUtils; import org.chromium.ui.base.ViewUtils;
...@@ -125,9 +126,7 @@ public class NewTabPageView extends FrameLayout { ...@@ -125,9 +126,7 @@ public class NewTabPageView extends FrameLayout {
assert manager.getSuggestionsSource() != null; assert manager.getSuggestionsSource() != null;
// Don't store a direct reference to the activity, because it might change later if the tab Runnable closeContextMenuCallback = TabUtils.getActivity(tab)::closeContextMenu;
// is reparented.
Runnable closeContextMenuCallback = () -> ((TabImpl) mTab).getActivity().closeContextMenu();
mContextMenuManager = new ContextMenuManager(mManager.getNavigationDelegate(), mContextMenuManager = new ContextMenuManager(mManager.getNavigationDelegate(),
mRecyclerView::setTouchEnabled, closeContextMenuCallback, mRecyclerView::setTouchEnabled, closeContextMenuCallback,
NewTabPage.CONTEXT_MENU_USER_ACTION_PREFIX); NewTabPage.CONTEXT_MENU_USER_ACTION_PREFIX);
......
...@@ -694,9 +694,12 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer { ...@@ -694,9 +694,12 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
} }
/** /**
* WARNING: This method is deprecated. Consider other ways such as passing the dependencies
* to the constructor, rather than accessing ChromeActivity from Tab and using getters.
* @return {@link ChromeActivity} that currently contains this {@link Tab} in its * @return {@link ChromeActivity} that currently contains this {@link Tab} in its
* {@link TabModel}. * {@link TabModel}.
*/ */
@Deprecated
public ChromeActivity<?> getActivity() { public ChromeActivity<?> getActivity() {
if (getWindowAndroid() == null) return null; if (getWindowAndroid() == null) return null;
Activity activity = ContextUtils.activityFromContext(getWindowAndroid().getContext().get()); Activity activity = ContextUtils.activityFromContext(getWindowAndroid().getContext().get());
......
...@@ -35,7 +35,7 @@ public final class TabParentIntent extends EmptyTabObserver implements UserData ...@@ -35,7 +35,7 @@ public final class TabParentIntent extends EmptyTabObserver implements UserData
} }
private TabParentIntent(Tab tab) { private TabParentIntent(Tab tab) {
mTab = (TabImpl) tab; mTab = tab;
mTab.addObserver(this); mTab.addObserver(this);
} }
...@@ -45,8 +45,8 @@ public final class TabParentIntent extends EmptyTabObserver implements UserData ...@@ -45,8 +45,8 @@ public final class TabParentIntent extends EmptyTabObserver implements UserData
// If the parent Tab belongs to another Activity, fire the Intent to bring it back. // If the parent Tab belongs to another Activity, fire the Intent to bring it back.
if (isSelected && mParentIntent != null if (isSelected && mParentIntent != null
&& ((TabImpl) tab).getActivity().getIntent() != mParentIntent) { && TabUtils.getActivity(tab).getIntent() != mParentIntent) {
((TabImpl) tab).getActivity().startActivity(mParentIntent); TabUtils.getActivity(tab).startActivity(mParentIntent);
} }
} }
......
// Copyright 2020 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.tab;
import android.app.Activity;
import androidx.annotation.Nullable;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.WindowAndroid;
/**
* Collection of utility methods that operates on Tab.
*/
public class TabUtils {
// Do not instantiate this class.
private TabUtils() {}
/**
* @return {@link Activity} associated with the given tab.
*/
@Nullable
public static Activity getActivity(Tab tab) {
WebContents webContents = tab != null ? tab.getWebContents() : null;
if (webContents == null || webContents.isDestroyed()) return null;
WindowAndroid window = webContents.getTopLevelNativeWindow();
return window != null ? window.getActivity().get() : null;
}
}
...@@ -15,7 +15,7 @@ import org.chromium.chrome.R; ...@@ -15,7 +15,7 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.infobar.InfoBarIdentifier; import org.chromium.chrome.browser.infobar.InfoBarIdentifier;
import org.chromium.chrome.browser.infobar.SimpleConfirmInfoBarBuilder; import org.chromium.chrome.browser.infobar.SimpleConfirmInfoBarBuilder;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabImpl; import org.chromium.chrome.browser.tab.TabUtils;
/** /**
* Installs AR DFM and ArCore runtimes. * Installs AR DFM and ArCore runtimes.
...@@ -89,7 +89,7 @@ public class ArCoreInstallUtils { ...@@ -89,7 +89,7 @@ public class ArCoreInstallUtils {
@ArCoreShim.Availability @ArCoreShim.Availability
int arCoreAvailability = getArCoreInstallStatus(); int arCoreAvailability = getArCoreInstallStatus();
final Activity activity = ((TabImpl) tab).getActivity(); final Activity activity = TabUtils.getActivity(tab);
String infobarText = null; String infobarText = null;
String buttonText = null; String buttonText = null;
switch (arCoreAvailability) { switch (arCoreAvailability) {
......
...@@ -14,7 +14,7 @@ import org.chromium.base.PackageUtils; ...@@ -14,7 +14,7 @@ import org.chromium.base.PackageUtils;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.chrome.browser.banners.AppData; import org.chromium.chrome.browser.banners.AppData;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabImpl; import org.chromium.chrome.browser.tab.TabUtils;
/** /**
* Provides functionality related to native Android apps for its C++ counterpart, * Provides functionality related to native Android apps for its C++ counterpart,
...@@ -33,9 +33,9 @@ class AddToHomescreenInstaller { ...@@ -33,9 +33,9 @@ class AddToHomescreenInstaller {
} else { } else {
launchIntent = appData.installIntent(); launchIntent = appData.installIntent();
} }
if (launchIntent != null && ((TabImpl) tab).getActivity() != null) { if (launchIntent != null && TabUtils.getActivity(tab) != null) {
try { try {
((TabImpl) tab).getActivity().startActivity(launchIntent); TabUtils.getActivity(tab).startActivity(launchIntent);
} catch (ActivityNotFoundException e) { } catch (ActivityNotFoundException e) {
Log.e(TAG, "Failed to install or open app : %s!", appData.packageName(), e); Log.e(TAG, "Failed to install or open app : %s!", appData.packageName(), e);
return false; return false;
......
...@@ -23,7 +23,6 @@ import org.chromium.chrome.browser.customtabs.CustomTabActivityTestRule; ...@@ -23,7 +23,6 @@ import org.chromium.chrome.browser.customtabs.CustomTabActivityTestRule;
import org.chromium.chrome.browser.flags.ChromeSwitches; import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.tab.SadTab; import org.chromium.chrome.browser.tab.SadTab;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabImpl;
import org.chromium.chrome.browser.tab.TabLaunchType; import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.test.ChromeActivityTestRule; import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
...@@ -105,7 +104,7 @@ public class ChromeHttpAuthHandlerTest { ...@@ -105,7 +104,7 @@ public class ChromeHttpAuthHandlerTest {
@MediumTest @MediumTest
@Restriction(Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE) @Restriction(Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE)
public void authDialogSuppressedOnBackgroundTab() throws Exception { public void authDialogSuppressedOnBackgroundTab() throws Exception {
TabImpl firstTab = (TabImpl) mActivityTestRule.getActivity().getActivityTab(); Tab firstTab = mActivityTestRule.getActivity().getActivityTab();
ChromeTabUtils.newTabFromMenu( ChromeTabUtils.newTabFromMenu(
InstrumentationRegistry.getInstrumentation(), mActivityTestRule.getActivity()); InstrumentationRegistry.getInstrumentation(), mActivityTestRule.getActivity());
// If the first tab was closed due to OOM, then just exit the test. // If the first tab was closed due to OOM, then just exit the test.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment