[Android] Move ChromeTabbedActivity to .app [chromium/src : main]

49 views
Skip to first unread message

Henrique Nakashima (Gerrit)

unread,
Jul 29, 2021, 5:34:18 PM7/29/21
to Theresa, amalov...@chromium.org, asvitki...@chromium.org, browser-comp...@chromium.org, carlos...@chromium.org, chili...@chromium.org, chromiumme...@microsoft.com, davidj...@chromium.org, dewitt...@chromium.org, dimich...@chromium.org, dominickn+...@chromium.org, donnd...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, fgorsk...@chromium.org, gogeral...@chromium.org, hanxi...@chromium.org, harringt...@chromium.org, iwells...@chromium.org, jinsukk...@chromium.org, lizeb+watch...@chromium.org, mattsimm...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, peconn...@chromium.org, petewi...@chromium.org, pkotwic...@chromium.org, poscia...@chromium.org, pshmako...@chromium.org, romax...@chromium.org, security-...@chromium.org, twelling...@chromium.org, webapks-...@chromium.org, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org, Brandon Wylie

Attention is currently required from: Theresa .

Henrique Nakashima would like Theresa to review this change.

View Change

[Android] Move ChromeTabbedActivity to .app

Move glue code into the chrome.browser.app package. This is an
important node that might break many cycles that were created when
ChromeActivity was moved.

Also move ChromeTabbedActivity2 to chrome.browser.app.

Bug: 1233155
Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
---
M chrome/android/DEPS
M chrome/android/chrome_java_sources.gni
M chrome/android/expectations/monochrome_public_bundle.AndroidManifest.expected
M chrome/android/expectations/trichrome_chrome_bundle.AndroidManifest.expected
M chrome/android/features/start_surface/internal/javatests/src/org/chromium/chrome/features/start_surface/InstantStartTest.java
M chrome/android/features/start_surface/internal/javatests/src/org/chromium/chrome/features/start_surface/StartSurfaceLayoutTest.java
M chrome/android/features/start_surface/internal/javatests/src/org/chromium/chrome/features/start_surface/StartSurfaceTest.java
M chrome/android/features/start_surface/internal/javatests/src/org/chromium/chrome/features/start_surface/StartSurfaceTestUtils.java
M chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_groups/TabGroupUtils.java
M chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/PriceAlertsMessageCardTest.java
M chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/PriceTrackingDialogTest.java
M chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabGridAccessibilityHelperTest.java
M chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabGridDialogTest.java
M chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabGridIphTest.java
M chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabGroupUiTest.java
M chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorTest.java
M chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabSwitcherMultiWindowTest.java
M chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabUiTestHelper.java
M chrome/android/features/vr/java/src/org/chromium/chrome/browser/vr/VrDelegate.java
M chrome/android/features/vr/java/src/org/chromium/chrome/browser/vr/VrShell.java
M chrome/android/features/vr/java/src/org/chromium/chrome/browser/vr/VrShellDelegate.java
M chrome/android/feed/core/javatests/src/org/chromium/chrome/browser/feed/FeedV2NewTabPageTest.java
M chrome/android/java/AndroidManifest.xml
M chrome/android/java/src/org/chromium/chrome/browser/AppIndexingUtil.java
M chrome/android/java/src/org/chromium/chrome/browser/DEPS
M chrome/android/java/src/org/chromium/chrome/browser/LaunchIntentDispatcher.java
M chrome/android/java/src/org/chromium/chrome/browser/TabbedModeTabDelegateFactory.java
R chrome/android/java/src/org/chromium/chrome/browser/app/ChromeTabbedActivity.java
R chrome/android/java/src/org/chromium/chrome/browser/app/ChromeTabbedActivity2.java
M chrome/android/java/src/org/chromium/chrome/browser/app/video_tutorials/VideoPlayerActivity.java
M chrome/android/java/src/org/chromium/chrome/browser/base/SplitChromeApplication.java
M chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUtils.java
M chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchQuickActionControl.java
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChrome.java
M chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java
M chrome/android/java/src/org/chromium/chrome/browser/download/DownloadInfoBarController.java
M chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
M chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImpl.java
M chrome/android/java/src/org/chromium/chrome/browser/history/HistoryContentManager.java
M chrome/android/java/src/org/chromium/chrome/browser/incognito/IncognitoNotificationServiceImpl.java
M chrome/android/java/src/org/chromium/chrome/browser/incognito/IncognitoUtils.java
M chrome/android/java/src/org/chromium/chrome/browser/init/StartupTabPreloader.java
M chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceChromeTabbedActivity.java
M chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManager.java
M chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManagerApi31.java
M chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiWindowUtils.java
M chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java
M chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelImpl.java
M chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/TabDelegate.java
M chrome/android/javatests/src/org/chromium/chrome/browser/ActivityTabProviderTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/ShareIntentTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/TabObserverTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/TabsOpenedFromExternalAppTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/TabsTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/app/appmenu/TabbedAppMenuTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/app/metrics/TabbedActivityLaunchCauseMetricsTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/app/metrics/TabbedActivityLaunchCauseMetricsUnitTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/app/tab_activity_glue/TabletPhoneLayoutChangeTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkPersonalizedSigninPromoDismissTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabTabPersistencePolicyTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/TrustedCdnPublisherUrlTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/dom_distiller/ReaderModeTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/UrlOverridingTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/firstrun/FirstRunIntegrationTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/gesturenav/NavigationSheetTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/homepage/HomepagePolicyIntegrationTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/incognito/IncognitoDataTestUtils.java
M chrome/android/javatests/src/org/chromium/chrome/browser/incognito/IncognitoTabLauncherTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/javascript/ModalCloseWatcherTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/jsdialog/JavascriptAppModalDialogTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/jsdialog/JavascriptTabModalDialogTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/PictureInPictureControllerTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/metrics/MainIntentBehaviorMetricsIntegrationTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/metrics/StartupLoadingMetricsTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/modaldialog/ChromeTabModalPresenterTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/multiwindow/MultiWindowIntegrationTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/multiwindow/MultiWindowTestHelper.java
M chrome/android/javatests/src/org/chromium/chrome/browser/multiwindow/MultiWindowUtilsTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/LocationBarTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/suggestions/SwitchToTabTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/suggestions/mostvisited/MostVisitedTilesTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/voice/AssistantVoiceSearchConsentUiTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerHomepageIntegrationTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/search_engines/DefaultSearchEnginePromoDialogTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/searchwidget/SearchActivityTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabUmaTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelImplTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelMergingTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/UndoTabModelTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/tasks/ReturnToChromeTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/LocationBarModelTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/top/AdaptiveToolbarTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/ui/MediaCaptureOverlayControllerTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/ui/system/StatusBarColorControllerTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/usage_stats/TabSuspensionTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/VrBrowserTransitionUtils.java
M chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkActivityTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java
M chrome/android/javatests/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetControllerTest.java
M chrome/android/javatests/src/org/chromium/chrome/test/smoke/ChromeSmokeTest.java
M chrome/android/junit/src/org/chromium/chrome/browser/app/tabmodel/TabPersistentStoreIntegrationTest.java
M chrome/android/junit/src/org/chromium/chrome/browser/firstrun/FirstRunIntegrationUnitTest.java
M chrome/android/junit/src/org/chromium/chrome/browser/toolbar/adaptive/OptionalNewTabButtonControllerActivityTest.java
M chrome/browser/DEPS
M chrome/browser/tabmodel/android/java/src/org/chromium/chrome/browser/tabmodel/IncognitoTabModelTest.java
M chrome/browser/ui/android/quickactionsearchwidget/java/src/org/chromium/chrome/browser/ui/quickactionsearchwidget/QuickActionSearchWidgetTestUtils.java
M chrome/test/android/browsertests_apk/src/org/chromium/android_browsertests_apk/ChromeBrowserTestsActivity.java
M chrome/test/android/javatests/src/org/chromium/chrome/test/ChromeTabbedActivityTestRule.java
M chrome/test/android/javatests/src/org/chromium/chrome/test/batch/BlankCTATabInitialStateRule.java
M chrome/test/android/javatests/src/org/chromium/chrome/test/util/ChromeTabUtils.java
M chrome/test/android/javatests/src/org/chromium/chrome/test/util/TabStripUtils.java
115 files changed, 206 insertions(+), 175 deletions(-)


To view, visit change 3054714. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
Gerrit-Change-Number: 3054714
Gerrit-PatchSet: 5
Gerrit-Owner: Henrique Nakashima <hnaka...@chromium.org>
Gerrit-Reviewer: Henrique Nakashima <hnaka...@chromium.org>
Gerrit-Reviewer: Theresa <twell...@chromium.org>
Gerrit-CC: Brandon Wylie <wyl...@chromium.org>
Gerrit-CC: Peter Beverloo <pe...@chromium.org>
Gerrit-Attention: Theresa <twell...@chromium.org>
Gerrit-MessageType: newchange

Henrique Nakashima (Gerrit)

unread,
Jul 29, 2021, 5:34:24 PM7/29/21
to amalov...@chromium.org, asvitki...@chromium.org, browser-comp...@chromium.org, carlos...@chromium.org, chili...@chromium.org, chromiumme...@microsoft.com, davidj...@chromium.org, dewitt...@chromium.org, dimich...@chromium.org, dominickn+...@chromium.org, donnd...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, fgorsk...@chromium.org, gogeral...@chromium.org, hanxi...@chromium.org, harringt...@chromium.org, iwells...@chromium.org, jinsukk...@chromium.org, lizeb+watch...@chromium.org, mattsimm...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, peconn...@chromium.org, petewi...@chromium.org, pkotwic...@chromium.org, poscia...@chromium.org, pshmako...@chromium.org, romax...@chromium.org, security-...@chromium.org, twelling...@chromium.org, webapks-...@chromium.org, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org, Theresa, Brandon Wylie, Chromium LUCI CQ, chromium...@chromium.org, Peter Beverloo

Attention is currently required from: Theresa .

View Change

    To view, visit change 3054714. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
    Gerrit-Change-Number: 3054714
    Gerrit-PatchSet: 5
    Gerrit-Owner: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Theresa <twell...@chromium.org>
    Gerrit-CC: Brandon Wylie <wyl...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-Attention: Theresa <twell...@chromium.org>
    Gerrit-Comment-Date: Thu, 29 Jul 2021 21:34:14 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Theresa (Gerrit)

    unread,
    Jul 30, 2021, 11:52:03 AM7/30/21
    to Michael Thiessen, amalov...@chromium.org, asvitki...@chromium.org, browser-comp...@chromium.org, carlos...@chromium.org, chili...@chromium.org, chromiumme...@microsoft.com, davidj...@chromium.org, dewitt...@chromium.org, dimich...@chromium.org, dominickn+...@chromium.org, donnd...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, fgorsk...@chromium.org, gogeral...@chromium.org, hanxi...@chromium.org, harringt...@chromium.org, iwells...@chromium.org, jinsukk...@chromium.org, lizeb+watch...@chromium.org, mattsimm...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, peconn...@chromium.org, petewi...@chromium.org, pkotwic...@chromium.org, poscia...@chromium.org, pshmako...@chromium.org, romax...@chromium.org, security-...@chromium.org, twelling...@chromium.org, webapks-...@chromium.org, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org, Henrique Nakashima

    Attention is currently required from: Michael Thiessen, Henrique Nakashima.

    Theresa would like Michael Thiessen to review this change authored by Henrique Nakashima.

    To view, visit change 3054714. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
    Gerrit-Change-Number: 3054714
    Gerrit-PatchSet: 5
    Gerrit-Owner: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Reviewer: Theresa <twell...@chromium.org>
    Gerrit-CC: Brandon Wylie <wyl...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-Attention: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Attention: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-MessageType: newchange

    Theresa (Gerrit)

    unread,
    Jul 30, 2021, 11:52:10 AM7/30/21
    to Henrique Nakashima, amalov...@chromium.org, asvitki...@chromium.org, browser-comp...@chromium.org, carlos...@chromium.org, chili...@chromium.org, chromiumme...@microsoft.com, davidj...@chromium.org, dewitt...@chromium.org, dimich...@chromium.org, dominickn+...@chromium.org, donnd...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, fgorsk...@chromium.org, gogeral...@chromium.org, hanxi...@chromium.org, harringt...@chromium.org, iwells...@chromium.org, jinsukk...@chromium.org, lizeb+watch...@chromium.org, mattsimm...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, peconn...@chromium.org, petewi...@chromium.org, pkotwic...@chromium.org, poscia...@chromium.org, pshmako...@chromium.org, romax...@chromium.org, security-...@chromium.org, twelling...@chromium.org, webapks-...@chromium.org, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org, Michael Thiessen, Brandon Wylie, Chromium LUCI CQ, chromium...@chromium.org, Peter Beverloo

    Attention is currently required from: Michael Thiessen, Henrique Nakashima.

    View Change

    1 comment:

    • File chrome/android/java/AndroidManifest.xml:

      • Patch Set #5, Line 458: 

        activity android:name="org.chromium.chrome.browser.app.ChromeTabbedActivity"
                    android:theme="@style/Theme.Chromium.TabbedMode"
                    android:exported="true"

        +mthiesse@ -- any issues you can see with this since the activity is exported?

        We don't really expect other apps to call us directly... but they could.

    To view, visit change 3054714. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
    Gerrit-Change-Number: 3054714
    Gerrit-PatchSet: 5
    Gerrit-Owner: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Reviewer: Theresa <twell...@chromium.org>
    Gerrit-CC: Brandon Wylie <wyl...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-Attention: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Attention: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Comment-Date: Fri, 30 Jul 2021 15:51:59 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Michael Thiessen (Gerrit)

    unread,
    Jul 30, 2021, 12:42:54 PM7/30/21
    to Ted Choc, amalov...@chromium.org, asvitki...@chromium.org, browser-comp...@chromium.org, carlos...@chromium.org, chili...@chromium.org, chromiumme...@microsoft.com, davidj...@chromium.org, dewitt...@chromium.org, dimich...@chromium.org, dominickn+...@chromium.org, donnd...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, fgorsk...@chromium.org, gogeral...@chromium.org, hanxi...@chromium.org, harringt...@chromium.org, iwells...@chromium.org, jinsukk...@chromium.org, lizeb+watch...@chromium.org, mattsimm...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, peconn...@chromium.org, petewi...@chromium.org, pkotwic...@chromium.org, poscia...@chromium.org, pshmako...@chromium.org, romax...@chromium.org, security-...@chromium.org, twelling...@chromium.org, webapks-...@chromium.org, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org, Henrique Nakashima, Theresa

    Attention is currently required from: Ted Choc, Theresa , Henrique Nakashima.

    Michael Thiessen would like Ted Choc to review this change authored by Henrique Nakashima.

    To view, visit change 3054714. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
    Gerrit-Change-Number: 3054714
    Gerrit-PatchSet: 5
    Gerrit-Owner: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Reviewer: Ted Choc <ted...@chromium.org>
    Gerrit-Reviewer: Theresa <twell...@chromium.org>
    Gerrit-CC: Brandon Wylie <wyl...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-Attention: Ted Choc <ted...@chromium.org>
    Gerrit-Attention: Theresa <twell...@chromium.org>

    Michael Thiessen (Gerrit)

    unread,
    Jul 30, 2021, 12:43:08 PM7/30/21
    to Henrique Nakashima, amalov...@chromium.org, asvitki...@chromium.org, browser-comp...@chromium.org, carlos...@chromium.org, chili...@chromium.org, chromiumme...@microsoft.com, davidj...@chromium.org, dewitt...@chromium.org, dimich...@chromium.org, dominickn+...@chromium.org, donnd...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, fgorsk...@chromium.org, gogeral...@chromium.org, hanxi...@chromium.org, harringt...@chromium.org, iwells...@chromium.org, jinsukk...@chromium.org, lizeb+watch...@chromium.org, mattsimm...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, peconn...@chromium.org, petewi...@chromium.org, pkotwic...@chromium.org, poscia...@chromium.org, pshmako...@chromium.org, romax...@chromium.org, security-...@chromium.org, twelling...@chromium.org, webapks-...@chromium.org, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org, Ted Choc, Theresa, Brandon Wylie, Chromium LUCI CQ, chromium...@chromium.org, Peter Beverloo

    Attention is currently required from: Ted Choc, Theresa , Henrique Nakashima.

    View Change

    1 comment:

    • File chrome/android/java/AndroidManifest.xml:

      • Patch Set #5, Line 458: 

        activity android:name="org.chromium.chrome.browser.app.ChromeTabbedActivity"
                    android:theme="@style/Theme.Chromium.TabbedMode"
                    android:exported="true"

      • +mthiesse@ -- any issues you can see with this since the activity is exported? […]

        Yeah, this is definitely not a safe change and will almost certainly break other apps as this is part of our public API.

        I *think* it will also lead to crashes after updates as the activity for your existing chrome tasks will no longer exist.

        We should definitely have Ted weigh in on this as I think he has the most experience renaming Activities in the past. I think there's a good chance the outcome will be that we cannot do this rename, but it might be possible to get away with creating an activity alias with the old componentname that targets the new one...

    To view, visit change 3054714. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
    Gerrit-Change-Number: 3054714
    Gerrit-PatchSet: 5
    Gerrit-Owner: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Reviewer: Ted Choc <ted...@chromium.org>
    Gerrit-Reviewer: Theresa <twell...@chromium.org>
    Gerrit-CC: Brandon Wylie <wyl...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-Attention: Ted Choc <ted...@chromium.org>
    Gerrit-Attention: Theresa <twell...@chromium.org>
    Gerrit-Attention: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Comment-Date: Fri, 30 Jul 2021 16:42:47 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Theresa <twell...@chromium.org>
    Gerrit-MessageType: comment

    Henrique Nakashima (Gerrit)

    unread,
    Jul 30, 2021, 2:30:56 PM7/30/21
    to amalov...@chromium.org, asvitki...@chromium.org, browser-comp...@chromium.org, carlos...@chromium.org, chili...@chromium.org, chromiumme...@microsoft.com, davidj...@chromium.org, dewitt...@chromium.org, dimich...@chromium.org, dominickn+...@chromium.org, donnd...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, fgorsk...@chromium.org, gogeral...@chromium.org, hanxi...@chromium.org, harringt...@chromium.org, iwells...@chromium.org, jinsukk...@chromium.org, lizeb+watch...@chromium.org, mattsimm...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, peconn...@chromium.org, petewi...@chromium.org, pkotwic...@chromium.org, poscia...@chromium.org, pshmako...@chromium.org, romax...@chromium.org, security-...@chromium.org, twelling...@chromium.org, webapks-...@chromium.org, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org, Ted Choc, Michael Thiessen, Theresa, Brandon Wylie, Chromium LUCI CQ, chromium...@chromium.org, Peter Beverloo

    Attention is currently required from: Ted Choc, Michael Thiessen, Theresa .

    View Change

    1 comment:

    • File chrome/android/java/AndroidManifest.xml:

      • Patch Set #5, Line 458: 

        activity android:name="org.chromium.chrome.browser.app.ChromeTabbedActivity"
                    android:theme="@style/Theme.Chromium.TabbedMode"
                    android:exported="true"

      • Yeah, this is definitely not a safe change and will almost certainly break other apps as this is par […]

        Ah, I see, tahkns for flagging. ChromeActivity was ok to move because it wasn't exported and not concrete. Creating an alias would get around the first problem, but probably not the second. Having some activities concretely be c.b.ChromeTabbedActivity and others c.b.app.ChromeTabbedActivity at the same time would likely create a host of other problems.

    To view, visit change 3054714. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
    Gerrit-Change-Number: 3054714
    Gerrit-PatchSet: 5
    Gerrit-Owner: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Reviewer: Ted Choc <ted...@chromium.org>
    Gerrit-Reviewer: Theresa <twell...@chromium.org>
    Gerrit-CC: Brandon Wylie <wyl...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-Attention: Ted Choc <ted...@chromium.org>
    Gerrit-Attention: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Attention: Theresa <twell...@chromium.org>
    Gerrit-Comment-Date: Fri, 30 Jul 2021 18:30:43 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Michael Thiessen <mthi...@chromium.org>

    Ted Choc (Gerrit)

    unread,
    Aug 3, 2021, 11:54:33 AM8/3/21
    to Henrique Nakashima, amalov...@chromium.org, asvitki...@chromium.org, browser-comp...@chromium.org, carlos...@chromium.org, chili...@chromium.org, chromiumme...@microsoft.com, davidj...@chromium.org, dewitt...@chromium.org, dimich...@chromium.org, dominickn+...@chromium.org, donnd...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, fgorsk...@chromium.org, gogeral...@chromium.org, hanxi...@chromium.org, harringt...@chromium.org, iwells...@chromium.org, jinsukk...@chromium.org, lizeb+watch...@chromium.org, mattsimm...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, peconn...@chromium.org, petewi...@chromium.org, pkotwic...@chromium.org, poscia...@chromium.org, pshmako...@chromium.org, romax...@chromium.org, security-...@chromium.org, twelling...@chromium.org, webapks-...@chromium.org, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org, Michael Thiessen, Theresa, Brandon Wylie, Chromium LUCI CQ, chromium...@chromium.org, Peter Beverloo

    Attention is currently required from: Michael Thiessen, Theresa , Henrique Nakashima.

    View Change

    1 comment:

    • File chrome/android/java/AndroidManifest.xml:

      • Patch Set #5, Line 458: 

        activity android:name="org.chromium.chrome.browser.app.ChromeTabbedActivity"
                    android:theme="@style/Theme.Chromium.TabbedMode"
                    android:exported="true"

      • Ah, I see, tahkns for flagging. […]

        I think an activity alias should work, but I'm not sure if that would result in different component names for the updated location and the older-then-aliased location.

        The classname on the java side would be the same, so that is OK.

        In the past where an alias didn't work for us, I recall us actually leaving a class in the old location that simply extends the other class.

        public class c.b.ChromeTabbedActivity extends c.b.app.ChromeTabbedActivity {}

        But that would definitely result in separate class types, so it is a bit wonky.

    To view, visit change 3054714. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
    Gerrit-Change-Number: 3054714
    Gerrit-PatchSet: 5
    Gerrit-Owner: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Reviewer: Ted Choc <ted...@chromium.org>
    Gerrit-Reviewer: Theresa <twell...@chromium.org>
    Gerrit-CC: Brandon Wylie <wyl...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-Attention: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Attention: Theresa <twell...@chromium.org>
    Gerrit-Attention: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Comment-Date: Tue, 03 Aug 2021 15:54:23 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Michael Thiessen <mthi...@chromium.org>
    Comment-In-Reply-To: Theresa <twell...@chromium.org>
    Comment-In-Reply-To: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-MessageType: comment

    Henrique Nakashima (Gerrit)

    unread,
    Aug 4, 2021, 2:25:42 PM8/4/21
    to amalov...@chromium.org, asvitki...@chromium.org, browser-comp...@chromium.org, carlos...@chromium.org, chili...@chromium.org, chromiumme...@microsoft.com, davidj...@chromium.org, dewitt...@chromium.org, dimich...@chromium.org, dominickn+...@chromium.org, donnd...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, fgorsk...@chromium.org, gogeral...@chromium.org, hanxi...@chromium.org, harringt...@chromium.org, iwells...@chromium.org, jinsukk...@chromium.org, lizeb+watch...@chromium.org, mattsimm...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, peconn...@chromium.org, petewi...@chromium.org, pkotwic...@chromium.org, poscia...@chromium.org, pshmako...@chromium.org, romax...@chromium.org, security-...@chromium.org, twelling...@chromium.org, webapks-...@chromium.org, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org, Ted Choc, Michael Thiessen, Theresa, Brandon Wylie, Chromium LUCI CQ, chromium...@chromium.org, Peter Beverloo

    Attention is currently required from: Ted Choc, Michael Thiessen, Theresa .

    View Change

    1 comment:

    • File chrome/android/java/AndroidManifest.xml:

      • Patch Set #5, Line 458: 

        activity android:name="org.chromium.chrome.browser.app.ChromeTabbedActivity"
                    android:theme="@style/Theme.Chromium.TabbedMode"
                    android:exported="true"

      • I think an activity alias should work, but I'm not sure if that would result in different component […]

        Yeah, I think the alias might make things more complicated instead of simpler, and the package dependency from chrome.browser to chrome.browser.app would still remain because of the alias.

        I'll drop this CL. Thanks for the review!

    To view, visit change 3054714. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
    Gerrit-Change-Number: 3054714
    Gerrit-PatchSet: 5
    Gerrit-Owner: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Reviewer: Ted Choc <ted...@chromium.org>
    Gerrit-Reviewer: Theresa <twell...@chromium.org>
    Gerrit-CC: Brandon Wylie <wyl...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-Attention: Ted Choc <ted...@chromium.org>
    Gerrit-Attention: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Attention: Theresa <twell...@chromium.org>
    Gerrit-Comment-Date: Wed, 04 Aug 2021 18:25:30 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Ted Choc <ted...@chromium.org>

    Henrique Nakashima (Gerrit)

    unread,
    Aug 4, 2021, 2:25:46 PM8/4/21
    to amalov...@chromium.org, asvitki...@chromium.org, browser-comp...@chromium.org, carlos...@chromium.org, chili...@chromium.org, chromiumme...@microsoft.com, davidj...@chromium.org, dewitt...@chromium.org, dimich...@chromium.org, dominickn+...@chromium.org, donnd...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, fgorsk...@chromium.org, gogeral...@chromium.org, hanxi...@chromium.org, harringt...@chromium.org, iwells...@chromium.org, jinsukk...@chromium.org, lizeb+watch...@chromium.org, mattsimm...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, peconn...@chromium.org, petewi...@chromium.org, pkotwic...@chromium.org, poscia...@chromium.org, pshmako...@chromium.org, romax...@chromium.org, security-...@chromium.org, twelling...@chromium.org, webapks-...@chromium.org, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org, Ted Choc, Michael Thiessen, Theresa, Brandon Wylie, Chromium LUCI CQ, chromium...@chromium.org, Peter Beverloo

    Henrique Nakashima abandoned this change.

    View Change

    Abandoned

    To view, visit change 3054714. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Idf89c19a5566defcf87890d81f136274364d5983
    Gerrit-Change-Number: 3054714
    Gerrit-PatchSet: 5
    Gerrit-Owner: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Henrique Nakashima <hnaka...@chromium.org>
    Gerrit-Reviewer: Michael Thiessen <mthi...@chromium.org>
    Gerrit-Reviewer: Ted Choc <ted...@chromium.org>
    Gerrit-Reviewer: Theresa <twell...@chromium.org>
    Gerrit-CC: Brandon Wylie <wyl...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-MessageType: abandon
    Reply all
    Reply to author
    Forward
    0 new messages