Finishing stopped CCT leaks activity |
|
Issue descriptionWhen CCT activity is finished in the stopped state, its Tab is not removed from PageLoadMetrics observers: → root class org.chromium.chrome.browser.metrics.PageLoadMetrics.sObservers → org.chromium.base.ObserverList.mObservers → java.util.ArrayList.elementData → java.lang.Object[15][4] → org.chromium.chrome.browser.customtabs.CustomTabActivity$PageLoadMetricsObserver.mTab → org.chromium.chrome.browser.tab.Tab.mFullscreenManager → org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager.mActivity → org.chromium.chrome.browser.customtabs.SeparateTaskCustomTabActivity This happens because CCT's Tab is removed from PageLoadMetrics observers by closeAllTabs(), which is called from CustomTabActivity.onStopWithNative() only when mIsClosing is true. But mIsClosing is set to true only in finishAndClose(), which is called when back or close button is pressed. I.e. when CCT activity is first stopped, and then finished (or destroyed) closeAllTabs() is not called, and Tab remains in PageLoadMetrics. Users can experience this leak with both same-task and separate-task CCTs when they are swiping them out from Android's recents.
,
Apr 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2f0dc80853e042b73036d403c6a9368b94e11e50 commit 2f0dc80853e042b73036d403c6a9368b94e11e50 Author: lizeb <lizeb@chromium.org> Date: Fri Apr 28 18:14:17 2017 customtabs: Don't cache the Tab in PageLoadMetricsObserver. Only the WebContents is needed, don't keep a reference to Tab, that keeps a reference to the Activity. BUG=715724 Review-Url: https://codereview.chromium.org/2847533004 Cr-Commit-Position: refs/heads/master@{#468060} [modify] https://crrev.com/2f0dc80853e042b73036d403c6a9368b94e11e50/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
,
Apr 28 2017
Thanks for the fix! Let's cherry-pick it to M59?
,
May 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d50d8466d0bdae0b5ae8af4108ecaa1376ed5f21 commit d50d8466d0bdae0b5ae8af4108ecaa1376ed5f21 Author: dskiba <dskiba@chromium.org> Date: Mon May 08 18:36:27 2017 Add tests to make sure closed CCT activities are not leaked. BUG=715724, 710612 Review-Url: https://codereview.chromium.org/2847263003 Cr-Commit-Position: refs/heads/master@{#470057} [modify] https://crrev.com/d50d8466d0bdae0b5ae8af4108ecaa1376ed5f21/base/android/java/src/org/chromium/base/ThreadUtils.java [modify] https://crrev.com/d50d8466d0bdae0b5ae8af4108ecaa1376ed5f21/chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java [modify] https://crrev.com/d50d8466d0bdae0b5ae8af4108ecaa1376ed5f21/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java [modify] https://crrev.com/d50d8466d0bdae0b5ae8af4108ecaa1376ed5f21/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java [modify] https://crrev.com/d50d8466d0bdae0b5ae8af4108ecaa1376ed5f21/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTestBase.java [modify] https://crrev.com/d50d8466d0bdae0b5ae8af4108ecaa1376ed5f21/chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java
,
May 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f4a47063c35627779a3e4d7464e1398bcbf6467 commit 3f4a47063c35627779a3e4d7464e1398bcbf6467 Author: olka <olka@chromium.org> Date: Tue May 09 12:53:52 2017 Revert of Add tests to make sure closed CCT activities are not leaked. (patchset #7 id:120001 of https://codereview.chromium.org/2847263003/ ) Reason for revert: org.chromium.chrome.browser.customtabs.CustomTabActivityTest#testClosedBackActivitiesNotLeaked flaky on chromium.linux/Android Tests See Issue 719909 Original issue's description: > Add tests to make sure closed CCT activities are not leaked. > > BUG=715724, 710612 > > Review-Url: https://codereview.chromium.org/2847263003 > Cr-Commit-Position: refs/heads/master@{#470057} > Committed: https://chromium.googlesource.com/chromium/src/+/d50d8466d0bdae0b5ae8af4108ecaa1376ed5f21 TBR=tedchoc@chromium.org,yusufo@chromium.org,torne@chromium.org,dskiba@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=715724, 710612 Review-Url: https://codereview.chromium.org/2866213003 Cr-Commit-Position: refs/heads/master@{#470295} [modify] https://crrev.com/3f4a47063c35627779a3e4d7464e1398bcbf6467/base/android/java/src/org/chromium/base/ThreadUtils.java [modify] https://crrev.com/3f4a47063c35627779a3e4d7464e1398bcbf6467/chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java [modify] https://crrev.com/3f4a47063c35627779a3e4d7464e1398bcbf6467/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java [modify] https://crrev.com/3f4a47063c35627779a3e4d7464e1398bcbf6467/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java [modify] https://crrev.com/3f4a47063c35627779a3e4d7464e1398bcbf6467/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTestBase.java [modify] https://crrev.com/3f4a47063c35627779a3e4d7464e1398bcbf6467/chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java |
|
►
Sign in to add a comment |
|
Comment 1 by lizeb@chromium.org
, Apr 27 2017Status: Started (was: Untriaged)