Issue metadata
Sign in to add a comment
|
New Custom Tab Activity session logic can kill other Activities |
||||||||||||||||||||||
Issue descriptionSay you have the following Activity Stack: 1: ActivityA 2: CustomTabActivity 3: ActivityB ActivityB launches a CustomTabsIntent with a new (or no) session. This Intent goes through the LaunchIntentDispatcher [1] where it has FLAG_ACTIVITY_SINGLE_TOP and FLAG_ACTIVITY_CLEAR_TOP added to it. It then gets launched to the existing CustomTabActivity, killing ActivityB. The CustomTabActivity realises that it can't handle the intent and launches a new CustomTabActivity [2], resulting in the following Activity Stack: 1: ActivityA 2: CustomTabActivity 3: CustomTabActivity The problem is that we check to see if the sessions matched *after* destroying any Activities on top. This also reveals the restriction that we don't really support multiple sessions per Task - say we had (I don't think it's possible to get into this state with the current bug, but say we fixed it): 1. ActivityA 2: CustomTabActivity session 1 3: CustomTabActivity session 2 4: ActivityB ActivityB launches a CustomTabsIntent re-using session 1, it's the most recent CustomTabActivity that is launched, which then launches a *new* CustomTabActivity. I think for the immediate bug, we should check whether sessions match before launching the SINGLE_TOP and CLEAR_TOP intents (since we'd ideally fix this for M72). Beyond that we should rethink session management. [1]: https://chromium.googlesource.com/chromium/src/+/84026b734855aa12557ce796bd38a2cd29aae3dd/chrome/android/java/src/org/chromium/chrome/browser/LaunchIntentDispatcher.java#302 [2]: https://chromium.googlesource.com/chromium/src/+/84026b734855aa12557ce796bd38a2cd29aae3dd/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java#570
,
Jan 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5 commit efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5 Author: Pavel Shmakov <pshmakov@google.com> Date: Fri Jan 11 15:33:36 2019 Check if there is a CCT with matching session in task before adding SINGLE_TOP and CLEAR_TOP flags The changes in https://crrev.com/c/1349653 included adding SINGLE_TOP and CLEAR_TOP flags to intents launching CCTs, and so activities above a CCT, should it be present in current task, were always getting destroyed. That shouldn't happen in cases when a client wants to launch a new CCT and to that end provides a different session token. An ideal solution would be to replace FLAG_ACTIVITY_CLEAR_TOP with FLAG_ACTIVITY_PREVIOUS_IS_TOP (which would prevent ChromeLauncherActivity getting in the way), and leave adding FLAG_ACTIVITY_CLEAR_TOP to the client. Unfortunately, FLAG_ACTIVITY_PREVIOUS_IS_TOP seems to be broken (https://b.corp.google.com/issues/122582732). The solution I propose is to keep a static map taskId -> session and use it to check if there is a CCT with matching session in the task prior to adding SINGLE_TOP and CLEAR_TOP flags. The mappings are not cleared in CCT's onStop or onDestroy to account for cases when the task is brought back to foreground by a new intent. They are only cleared on service connection termination. This should take care of this bug, however there could be potentially other side effects from the new behavior for existing clients (although, most likely they would be due to misuse of APIs). For that reason, this CL also restricts the new behavior to TWAs. For M73 we plan to remove this restriction. Bug: 918857 Change-Id: I5af4f2d9d3f9ae1bc57a9b21e5760b96575aa548 Reviewed-on: https://chromium-review.googlesource.com/c/1403119 Commit-Queue: Pavel Shmakov <pshmakov@chromium.org> Reviewed-by: Peter Conn <peconn@chromium.org> Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#622011} [modify] https://crrev.com/efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5/chrome/android/java/src/org/chromium/chrome/browser/LaunchIntentDispatcher.java [modify] https://crrev.com/efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentHandler.java [modify] https://crrev.com/efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentUtils.java [modify] https://crrev.com/efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java [modify] https://crrev.com/efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
,
Jan 11
Requesting a merge to 72. This fixes a bug in interaction between AGSA and CCT. Effectively, this reverts the change in intent handling behavior introduced in 72 for all CCT clients, except for the Trusted Web Activities (for TWAs that change is important), as well as fixing the bug in the new behavior.
,
Jan 11
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 11
For additional context, the change was extensively tested by AGSA engineers, who verified that the bug is gone, and didn't find any new issues in AGSA-CCT interaction. Given that, and the fact it restores the pre-M72 behavior for all existing CCT clients (TWAs have not been yet launched at the moment), we consider the change to be safe.
,
Jan 11
Pls update bug with canary result on Monday morning. If change looks good in canary, I will approve merge to M72. Thank you.
,
Jan 15
Pls verify this change on 73.0.3671.2+ and update bug with canary result today. Thank you.
,
Jan 15
We can verify the fix works with 73.0.3671.2.
,
Jan 15
Sorry, we have verified that it works with 73.0.3671.2.
,
Jan 15
Approving merge to M72 branch 3626 based on comments #3, #5 & #9. Please merge ASAP so we can pick it up for tomorrow's beta release. Thank you.
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e85fa5c8c46eda6cb8995032977a900a0f67f4c Commit: 8e85fa5c8c46eda6cb8995032977a900a0f67f4c Author: pshmakov@google.com Commiter: pshmakov@chromium.org Date: 2019-01-15 15:26:08 +0000 UTC Check if there is a CCT with matching session in task before adding SINGLE_TOP and CLEAR_TOP flags The changes in https://crrev.com/c/1349653 included adding SINGLE_TOP and CLEAR_TOP flags to intents launching CCTs, and so activities above a CCT, should it be present in current task, were always getting destroyed. That shouldn't happen in cases when a client wants to launch a new CCT and to that end provides a different session token. An ideal solution would be to replace FLAG_ACTIVITY_CLEAR_TOP with FLAG_ACTIVITY_PREVIOUS_IS_TOP (which would prevent ChromeLauncherActivity getting in the way), and leave adding FLAG_ACTIVITY_CLEAR_TOP to the client. Unfortunately, FLAG_ACTIVITY_PREVIOUS_IS_TOP seems to be broken (https://b.corp.google.com/issues/122582732). The solution I propose is to keep a static map taskId -> session and use it to check if there is a CCT with matching session in the task prior to adding SINGLE_TOP and CLEAR_TOP flags. The mappings are not cleared in CCT's onStop or onDestroy to account for cases when the task is brought back to foreground by a new intent. They are only cleared on service connection termination. This should take care of this bug, however there could be potentially other side effects from the new behavior for existing clients (although, most likely they would be due to misuse of APIs). For that reason, this CL also restricts the new behavior to TWAs. For M73 we plan to remove this restriction. Bug: 918857 Change-Id: I5af4f2d9d3f9ae1bc57a9b21e5760b96575aa548 Reviewed-on: https://chromium-review.googlesource.com/c/1403119 Commit-Queue: Pavel Shmakov <pshmakov@chromium.org> Reviewed-by: Peter Conn <peconn@chromium.org> Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#622011}(cherry picked from commit efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5) Reviewed-on: https://chromium-review.googlesource.com/c/1411999 Reviewed-by: Pavel Shmakov <pshmakov@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#687} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e85fa5c8c46eda6cb8995032977a900a0f67f4c commit 8e85fa5c8c46eda6cb8995032977a900a0f67f4c Author: Pavel Shmakov <pshmakov@google.com> Date: Tue Jan 15 15:26:08 2019 Check if there is a CCT with matching session in task before adding SINGLE_TOP and CLEAR_TOP flags The changes in https://crrev.com/c/1349653 included adding SINGLE_TOP and CLEAR_TOP flags to intents launching CCTs, and so activities above a CCT, should it be present in current task, were always getting destroyed. That shouldn't happen in cases when a client wants to launch a new CCT and to that end provides a different session token. An ideal solution would be to replace FLAG_ACTIVITY_CLEAR_TOP with FLAG_ACTIVITY_PREVIOUS_IS_TOP (which would prevent ChromeLauncherActivity getting in the way), and leave adding FLAG_ACTIVITY_CLEAR_TOP to the client. Unfortunately, FLAG_ACTIVITY_PREVIOUS_IS_TOP seems to be broken (https://b.corp.google.com/issues/122582732). The solution I propose is to keep a static map taskId -> session and use it to check if there is a CCT with matching session in the task prior to adding SINGLE_TOP and CLEAR_TOP flags. The mappings are not cleared in CCT's onStop or onDestroy to account for cases when the task is brought back to foreground by a new intent. They are only cleared on service connection termination. This should take care of this bug, however there could be potentially other side effects from the new behavior for existing clients (although, most likely they would be due to misuse of APIs). For that reason, this CL also restricts the new behavior to TWAs. For M73 we plan to remove this restriction. Bug: 918857 Change-Id: I5af4f2d9d3f9ae1bc57a9b21e5760b96575aa548 Reviewed-on: https://chromium-review.googlesource.com/c/1403119 Commit-Queue: Pavel Shmakov <pshmakov@chromium.org> Reviewed-by: Peter Conn <peconn@chromium.org> Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#622011}(cherry picked from commit efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5) Reviewed-on: https://chromium-review.googlesource.com/c/1411999 Reviewed-by: Pavel Shmakov <pshmakov@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#687} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/8e85fa5c8c46eda6cb8995032977a900a0f67f4c/chrome/android/java/src/org/chromium/chrome/browser/LaunchIntentDispatcher.java [modify] https://crrev.com/8e85fa5c8c46eda6cb8995032977a900a0f67f4c/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentHandler.java [modify] https://crrev.com/8e85fa5c8c46eda6cb8995032977a900a0f67f4c/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentUtils.java [modify] https://crrev.com/8e85fa5c8c46eda6cb8995032977a900a0f67f4c/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java [modify] https://crrev.com/8e85fa5c8c46eda6cb8995032977a900a0f67f4c/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
,
Jan 15
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec5b72149fa9217869aa3e5b2c293f6d050cb347 commit ec5b72149fa9217869aa3e5b2c293f6d050cb347 Author: Pavel Shmakov <pshmakov@chromium.org> Date: Tue Jan 15 22:01:11 2019 Revert "Check if there is a CCT with matching session in task before adding SINGLE_TOP and CLEAR_TOP flags" This reverts commit 8e85fa5c8c46eda6cb8995032977a900a0f67f4c. Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=922167 Original change's description: > Check if there is a CCT with matching session in task before adding SINGLE_TOP and CLEAR_TOP flags > > The changes in https://crrev.com/c/1349653 included adding SINGLE_TOP > and CLEAR_TOP flags to intents launching CCTs, and so activities above > a CCT, should it be present in current task, were always getting > destroyed. That shouldn't happen in cases when a client wants to launch > a new CCT and to that end provides a different session token. > > An ideal solution would be to replace FLAG_ACTIVITY_CLEAR_TOP with > FLAG_ACTIVITY_PREVIOUS_IS_TOP (which would prevent ChromeLauncherActivity > getting in the way), and leave adding FLAG_ACTIVITY_CLEAR_TOP to the > client. Unfortunately, FLAG_ACTIVITY_PREVIOUS_IS_TOP seems to be broken > (https://b.corp.google.com/issues/122582732). > > The solution I propose is to keep a static map taskId -> session and use > it to check if there is a CCT with matching session in the task prior > to adding SINGLE_TOP and CLEAR_TOP flags. > > The mappings are not cleared in CCT's onStop or onDestroy to account > for cases when the task is brought back to foreground by a new intent. > They are only cleared on service connection termination. > > This should take care of this bug, however there could be potentially > other side effects from the new behavior for existing clients (although, > most likely they would be due to misuse of APIs). For that reason, this > CL also restricts the new behavior to TWAs. For M73 we plan to remove > this restriction. > > Bug: 918857 > Change-Id: I5af4f2d9d3f9ae1bc57a9b21e5760b96575aa548 > Reviewed-on: https://chromium-review.googlesource.com/c/1403119 > Commit-Queue: Pavel Shmakov <pshmakov@chromium.org> > Reviewed-by: Peter Conn <peconn@chromium.org> > Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org> > Reviewed-by: Avi Drissman <avi@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#622011}(cherry picked from commit efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5) > Reviewed-on: https://chromium-review.googlesource.com/c/1411999 > Reviewed-by: Pavel Shmakov <pshmakov@chromium.org> > Cr-Commit-Position: refs/branch-heads/3626@{#687} > Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} TBR=peconn@chromium.org,pshmakov@chromium.org Change-Id: I7714dacd8beb2c586347d40216e46f52695f0a9a No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 918857 Reviewed-on: https://chromium-review.googlesource.com/c/1412823 Reviewed-by: Pavel Shmakov <pshmakov@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#701} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/ec5b72149fa9217869aa3e5b2c293f6d050cb347/chrome/android/java/src/org/chromium/chrome/browser/LaunchIntentDispatcher.java [modify] https://crrev.com/ec5b72149fa9217869aa3e5b2c293f6d050cb347/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentHandler.java [modify] https://crrev.com/ec5b72149fa9217869aa3e5b2c293f6d050cb347/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentUtils.java [modify] https://crrev.com/ec5b72149fa9217869aa3e5b2c293f6d050cb347/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java [modify] https://crrev.com/ec5b72149fa9217869aa3e5b2c293f6d050cb347/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec5b72149fa9217869aa3e5b2c293f6d050cb347 Commit: ec5b72149fa9217869aa3e5b2c293f6d050cb347 Author: pshmakov@chromium.org Commiter: pshmakov@chromium.org Date: 2019-01-15 22:01:11 +0000 UTC Revert "Check if there is a CCT with matching session in task before adding SINGLE_TOP and CLEAR_TOP flags" This reverts commit 8e85fa5c8c46eda6cb8995032977a900a0f67f4c. Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=922167 Original change's description: > Check if there is a CCT with matching session in task before adding SINGLE_TOP and CLEAR_TOP flags > > The changes in https://crrev.com/c/1349653 included adding SINGLE_TOP > and CLEAR_TOP flags to intents launching CCTs, and so activities above > a CCT, should it be present in current task, were always getting > destroyed. That shouldn't happen in cases when a client wants to launch > a new CCT and to that end provides a different session token. > > An ideal solution would be to replace FLAG_ACTIVITY_CLEAR_TOP with > FLAG_ACTIVITY_PREVIOUS_IS_TOP (which would prevent ChromeLauncherActivity > getting in the way), and leave adding FLAG_ACTIVITY_CLEAR_TOP to the > client. Unfortunately, FLAG_ACTIVITY_PREVIOUS_IS_TOP seems to be broken > (https://b.corp.google.com/issues/122582732). > > The solution I propose is to keep a static map taskId -> session and use > it to check if there is a CCT with matching session in the task prior > to adding SINGLE_TOP and CLEAR_TOP flags. > > The mappings are not cleared in CCT's onStop or onDestroy to account > for cases when the task is brought back to foreground by a new intent. > They are only cleared on service connection termination. > > This should take care of this bug, however there could be potentially > other side effects from the new behavior for existing clients (although, > most likely they would be due to misuse of APIs). For that reason, this > CL also restricts the new behavior to TWAs. For M73 we plan to remove > this restriction. > > Bug: 918857 > Change-Id: I5af4f2d9d3f9ae1bc57a9b21e5760b96575aa548 > Reviewed-on: https://chromium-review.googlesource.com/c/1403119 > Commit-Queue: Pavel Shmakov <pshmakov@chromium.org> > Reviewed-by: Peter Conn <peconn@chromium.org> > Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org> > Reviewed-by: Avi Drissman <avi@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#622011}(cherry picked from commit efb52c5ed5179145f1ed02a3a9d74c0abb8a74f5) > Reviewed-on: https://chromium-review.googlesource.com/c/1411999 > Reviewed-by: Pavel Shmakov <pshmakov@chromium.org> > Cr-Commit-Position: refs/branch-heads/3626@{#687} > Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} TBR=peconn@chromium.org,pshmakov@chromium.org Change-Id: I7714dacd8beb2c586347d40216e46f52695f0a9a No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 918857 Reviewed-on: https://chromium-review.googlesource.com/c/1412823 Reviewed-by: Pavel Shmakov <pshmakov@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#701} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Jan 17
(5 days ago)
,
Jan 17
(5 days ago)
Requesting a re-merge for 72 of this change, including the fix of 922167. The fix should land in canary 73.0.3674.0, we'll let it bake there for a day, and if it looks good we'd like to merge tomorrow.
,
Jan 17
(5 days ago)
This bug requires manual review: We are only 11 days from stable. Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 17
(5 days ago)
Pls update bug with canary result on Monday morning, 01/21. Pls note we can take this change in for M72 only if it is fully safe to merge as M72 is going to stable VERY soon.
,
Yesterday
(45 hours ago)
The bug that caused the revert is fixed and no longer seen in crash reports for canary since 73.0.3674.0, and no new crashes related to this change are seen. We consider this change ready for re-merging.
,
Yesterday
(38 hours ago)
,
Yesterday
(38 hours ago)
To summarize, the CL reverts the change that caused a regression in M72. Revert is applied for all CCT clients, except for the Trusted Web Activities (for which that change is essential). Trusted Web Activities are hidden behind a flag.
,
Yesterday
(38 hours ago)
Approving merge for https://chromium-review.googlesource.com/c/chromium/src/+/1417533 to M72 branch 3626 based on comment #10, #20 and #22 and per offline chat with pshmakov@.
,
Yesterday
(38 hours ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/838c28f531138fe8e09bc841d981ade599818521 commit 838c28f531138fe8e09bc841d981ade599818521 Author: Pavel Shmakov <pshmakov@google.com> Date: Mon Jan 21 16:22:26 2019 Check if there is a CCT with matching session in task before adding SINGLE_TOP and CLEAR_TOP flags This cherry-pick includes the fix for https://crbug.com/922944 The changes in https://crrev.com/c/1349653 included adding SINGLE_TOP and CLEAR_TOP flags to intents launching CCTs, and so activities above a CCT, should it be present in current task, were always getting destroyed. That shouldn't happen in cases when a client wants to launch a new CCT and to that end provides a different session token. An ideal solution would be to replace FLAG_ACTIVITY_CLEAR_TOP with FLAG_ACTIVITY_PREVIOUS_IS_TOP (which would prevent ChromeLauncherActivity getting in the way), and leave adding FLAG_ACTIVITY_CLEAR_TOP to the client. Unfortunately, FLAG_ACTIVITY_PREVIOUS_IS_TOP seems to be broken (https://b.corp.google.com/issues/122582732). The solution I propose is to keep a static map taskId -> session and use it to check if there is a CCT with matching session in the task prior to adding SINGLE_TOP and CLEAR_TOP flags. The mappings are not cleared in CCT's onStop or onDestroy to account for cases when the task is brought back to foreground by a new intent. They are only cleared on service connection termination. This should take care of this bug, however there could be potentially other side effects from the new behavior for existing clients (although, most likely they would be due to misuse of APIs). For that reason, this CL also restricts the new behavior to TWAs. For M73 we plan to remove this restriction. TBR: peconn@chromium.org Bug: 918857 Change-Id: I6cc26d5315011e4a99bf63c7b6f32fdbdac74c70 Reviewed-on: https://chromium-review.googlesource.com/c/1417533 Reviewed-by: Pavel Shmakov <pshmakov@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#743} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/838c28f531138fe8e09bc841d981ade599818521/chrome/android/java/src/org/chromium/chrome/browser/LaunchIntentDispatcher.java [modify] https://crrev.com/838c28f531138fe8e09bc841d981ade599818521/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentHandler.java [modify] https://crrev.com/838c28f531138fe8e09bc841d981ade599818521/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentUtils.java [modify] https://crrev.com/838c28f531138fe8e09bc841d981ade599818521/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java [modify] https://crrev.com/838c28f531138fe8e09bc841d981ade599818521/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
,
Yesterday
(38 hours ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/838c28f531138fe8e09bc841d981ade599818521 Commit: 838c28f531138fe8e09bc841d981ade599818521 Author: pshmakov@google.com Commiter: pshmakov@chromium.org Date: 2019-01-21 16:22:26 +0000 UTC Check if there is a CCT with matching session in task before adding SINGLE_TOP and CLEAR_TOP flags This cherry-pick includes the fix for https://crbug.com/922944 The changes in https://crrev.com/c/1349653 included adding SINGLE_TOP and CLEAR_TOP flags to intents launching CCTs, and so activities above a CCT, should it be present in current task, were always getting destroyed. That shouldn't happen in cases when a client wants to launch a new CCT and to that end provides a different session token. An ideal solution would be to replace FLAG_ACTIVITY_CLEAR_TOP with FLAG_ACTIVITY_PREVIOUS_IS_TOP (which would prevent ChromeLauncherActivity getting in the way), and leave adding FLAG_ACTIVITY_CLEAR_TOP to the client. Unfortunately, FLAG_ACTIVITY_PREVIOUS_IS_TOP seems to be broken (https://b.corp.google.com/issues/122582732). The solution I propose is to keep a static map taskId -> session and use it to check if there is a CCT with matching session in the task prior to adding SINGLE_TOP and CLEAR_TOP flags. The mappings are not cleared in CCT's onStop or onDestroy to account for cases when the task is brought back to foreground by a new intent. They are only cleared on service connection termination. This should take care of this bug, however there could be potentially other side effects from the new behavior for existing clients (although, most likely they would be due to misuse of APIs). For that reason, this CL also restricts the new behavior to TWAs. For M73 we plan to remove this restriction. TBR: peconn@chromium.org Bug: 918857 Change-Id: I6cc26d5315011e4a99bf63c7b6f32fdbdac74c70 Reviewed-on: https://chromium-review.googlesource.com/c/1417533 Reviewed-by: Pavel Shmakov <pshmakov@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#743} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Yesterday
(37 hours ago)
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 Deleted