New issue
Advanced search Search tips

Issue 916518 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task


Sign in to add a comment

Extract CustomTabActivityTabController out of CustomTabActivity

Project Member Reported by pshmakov@google.com, Dec 19

Issue description

Extract the low-level code working directly with Tab, Webcontents and related entities out of CustomTabActivity into a dedicated class; make it unit-testable and write some tests.

This is a part of CustomTabActivity refactoring plan described in
https://docs.google.com/document/d/1E9u-9eSFjLwVnUUIekhurmKk-8vsSDxek7cZiFd6ToY

 
Blocking: 916528
Blocking: 916531
Blocking: 916536
Blocking: 916516
Components: UI>Browser>Mobile>CustomTabs
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9129b970f8d600e00d05d1a6a85658fbe21c8eeb

commit 9129b970f8d600e00d05d1a6a85658fbe21c8eeb
Author: Pavel Shmakov <pshmakov@google.com>
Date: Tue Jan 15 11:27:45 2019

Extract CustomTabActivityTabController out of CustomTabActivity

A class responsible for working directly with Tab, Webcontents and
related classes is extracted out of CustomTabActivity code. It is made
unit-testable, and a few tests are written.

The only change in the logic is that CustomTabActivityTabController
updates mTab field whenever the website changes the currently seen tab.
This is in contrast with current code where mMainTab is always the
initial tab, which leaves some room for confusion and mistakes.

A few minor changes were required outside of CTA and the new classes:

- Make some methods non-static for testability.
- Remove a few "final" modifiers for testability. We may try
mockito-inline to make keeping "final" possible.
- Replace package visibility with public in a few places. If we want
to have any package structure inside the customtabs package, package
visibility has to go.
- Moved dagger component creation to an earlier time to allow receiving
onPreInflationStartup event.

Change-Id: Idc81e8435f6e719b621ed08082423e4bd5eae080

Bug:  916518 
Change-Id: Idc81e8435f6e719b621ed08082423e4bd5eae080
Reviewed-on: https://chromium-review.googlesource.com/c/1382496
Commit-Queue: Pavel Shmakov <pshmakov@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622813}
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/WebContentsFactory.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionDataProvider.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/view/TrustedWebActivityToolbarView.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabBrowserControlsVisibilityDelegate.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabNavigationEventObserver.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabObserver.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabTabPersistencePolicy.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/FirstMeaningfulPaintObserver.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/PageLoadMetricsObserver.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/PaymentHandlerActivity.java
[add] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityTabController.java
[add] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityTabFactory.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dependency_injection/CustomTabActivityComponent.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/dependency_injection/ChromeActivityCommonsModule.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/dependency_injection/ChromeAppModule.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/AsyncTabParamsManager.java
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/java_sources.gni
[modify] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabTabPersistenceIntegrationTest.java
[add] https://crrev.com/9129b970f8d600e00d05d1a6a85658fbe21c8eeb/chrome/android/junit/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityTabControllerTest.java

Status: Fixed (was: Started)

Sign in to add a comment