New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Task

Blocking:
issue 882436



Sign in to add a comment
link

Issue 904826: Refactor dynamic module code out of CustomTabActivity

Reported by mvanouwe...@chromium.org, Nov 13 Project Member

Issue description

The amount of code for dynamic modules is growing, and some of it is complex and brittle. It would be nice to pull some of it out of CustomTabActivity, perhaps using Dagger dependency injection and ActivityLifecycleDispatcher. I think pshmakov would be the right person to discuss the design with, and also a good reviewer.
 

Comment 1 by mvanouwe...@chromium.org, Nov 13

Blocking: 882436

Comment 2 by bugdroid1@chromium.org, Dec 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ea010eb1b39089eee142e010e2f92ba93873d82

commit 6ea010eb1b39089eee142e010e2f92ba93873d82
Author: Anna Malova <amalova@chromium.org>
Date: Tue Dec 11 20:28:27 2018

Move CCT Dynamic Module code out of CustomTabActivity.

Introduce DynamicModuleCoordinator which is instantiated via DI only
if Intent has correct dynamic module ComponentName.

ActivityDelegate is responsible of notifying dynamic module
on ActivityLifecycle events now.

Bug:  904826 
Change-Id: I2e73cdb296db9aab83031bd4c8bf99457bc8973f
Reviewed-on: https://chromium-review.googlesource.com/c/1356961
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Pavel Shmakov <pshmakov@chromium.org>
Commit-Queue: Anna Malova <amalova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615645}
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabBottomBarDelegate.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabTopBarDelegate.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dependency_injection/CustomTabActivityComponent.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/ActivityDelegate.java
[add] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/ActivityDelegatePostMessageBackend.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/ActivityHostImpl.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/DynamicModuleConstants.java
[add] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/DynamicModuleCoordinator.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/ModuleEntryPoint.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/init/ActivityLifecycleDispatcher.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/init/AsyncInitializationActivity.java
[add] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/lifecycle/SaveInstanceStateObserver.java
[add] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java/src/org/chromium/chrome/browser/lifecycle/WindowFocusChangedObserver.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/java_sources.gni
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
[modify] https://crrev.com/6ea010eb1b39089eee142e010e2f92ba93873d82/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsDynamicModuleNavigationTest.java

Comment 3 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2617cedd397c340eada91491e9f9fdf9dfad0b7f

commit 2617cedd397c340eada91491e9f9fdf9dfad0b7f
Author: Anna Malova <amalova@chromium.org>
Date: Fri Dec 14 12:23:47 2018

Separate CCT dynamic module related tests from CustomTabActivityTests.

Bug:  904826 
Change-Id: Icce53f66b5d6afefdc639a396a42b52f1b15a599
Reviewed-on: https://chromium-review.googlesource.com/c/1371879
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Commit-Queue: Anna Malova <amalova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616645}
[modify] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
[modify] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java
[modify] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/DynamicModuleCoordinator.java
[modify] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/android/java_sources.gni
[modify] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
[rename] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModuleLoaderTest.java
[rename] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModuleNavigationTest.java
[add] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModulePostMessageTest.java
[rename] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModuleTestUtils.java
[add] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModuleUITest.java
[modify] https://crrev.com/2617cedd397c340eada91491e9f9fdf9dfad0b7f/chrome/test/android/javatests/src/org/chromium/chrome/test/util/ChromeTabUtils.java

Comment 4 by amalova@chromium.org, Dec 14

Status: Fixed (was: Assigned)

Comment 5 by bugdroid1@chromium.org, Jan 22

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9344bb7cec42eabc83d8cabc87f31c4458f59464

commit 9344bb7cec42eabc83d8cabc87f31c4458f59464
Author: Anna Malova <amalova@chromium.org>
Date: Tue Jan 22 14:23:02 2019

Use CustomTabActivityTabController to loadUrl in Tab instead of casting ChromeActivity to CustomTabActivity.

Bug:  904826 
Change-Id: I7edb6bec4cbfabfbba43e9e27a453c579410e98a
Reviewed-on: https://chromium-review.googlesource.com/c/1422379
Reviewed-by: Pavel Shmakov <pshmakov@chromium.org>
Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Commit-Queue: Anna Malova <amalova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624770}
[modify] https://crrev.com/9344bb7cec42eabc83d8cabc87f31c4458f59464/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
[modify] https://crrev.com/9344bb7cec42eabc83d8cabc87f31c4458f59464/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/DynamicModuleCoordinator.java

Sign in to add a comment