Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2386753002:
Only allow one instance of Preferences to show at a time (Closed)
DescriptionOnly allow one instance of Preferences to show at a time in Android N+ multi-window
BUG=603343, 651317
Committed: https://crrev.com/82ccb5f152957e3f6d61e4bb02160868d7abaf87
Cr-Commit-Position: refs/heads/master@{#422546}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Only allow one instance of Preferences to show at a time in Android N+ multi-window #Messages
Total messages: 19 (12 generated)
The CQ bit was checked by twellington@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
twellington@chromium.org changed reviewers: + tedchoc@chromium.org
ptal There are videos of how this behaves in crbug.com/603343
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2386753002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/preferences/Preferences.java (right): https://codereview.chromium.org/2386753002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/preferences/Preferences.java:189: sPausedInstance = this; should we set sPausedInstance = null in onStop (or onDestroy if not)? I think this might cause a memory leak otherwise.
Description was changed from ========== Only allow one instance of Preferences to show at a time In Android N+ multi-window mode, it was previously possible to have two instances of Prefences showing at the same time. Keep track of the Preference instance when it is paused so that it can be properly finished if a second instance is created. BUG=603343, 651317 ========== to ========== Only allow one instance of Preferences to show at a time in Android N+ multi-window BUG=603343, 651317 ==========
The CQ bit was checked by twellington@chromium.org to run a CQ dry run
https://codereview.chromium.org/2386753002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/preferences/Preferences.java (right): https://codereview.chromium.org/2386753002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/preferences/Preferences.java:189: sPausedInstance = this; On 2016/10/03 20:23:38, Ted C wrote: > should we set sPausedInstance = null in onStop (or onDestroy if not)? > > I think this might cause a memory leak otherwise. Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by twellington@chromium.org
The CQ bit was checked by twellington@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Only allow one instance of Preferences to show at a time in Android N+ multi-window BUG=603343, 651317 ========== to ========== Only allow one instance of Preferences to show at a time in Android N+ multi-window BUG=603343, 651317 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Only allow one instance of Preferences to show at a time in Android N+ multi-window BUG=603343, 651317 ========== to ========== Only allow one instance of Preferences to show at a time in Android N+ multi-window BUG=603343, 651317 Committed: https://crrev.com/82ccb5f152957e3f6d61e4bb02160868d7abaf87 Cr-Commit-Position: refs/heads/master@{#422546} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/82ccb5f152957e3f6d61e4bb02160868d7abaf87 Cr-Commit-Position: refs/heads/master@{#422546} |
