New issue
Advanced search Search tips

Issue 904826 link

Starred by 1 user
Status: Fixed
Owner:
amalova@chromium.org
Closed: Dec 14
Cc:
peconn@chromium.org
pshmakov@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Task

Blocking:
issue 882436



Sign in to add a comment

Refactor dynamic module code out of CustomTabActivity

Project Member Reported by mvanouwe...@chromium.org, Nov 13 
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
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 11 

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
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 14 

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)
Project Member

Comment 5 by bugdroid1@chromium.org, Today (17 hours ago) 

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