Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2595933002:
Change assert in RecentTabsPage.destroy() (Closed)
|
Created:
4 years ago by finkm Modified:
3 years, 11 months ago Reviewers:
Bernhard Bauer CC:
agrieve+watch_chromium.org, chromium-reviews, ntp-dev+reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionChange assert in RecentTabsPage.destroy(). It always failed when the RecentTabsPage was closed without opening a recent tab.
BUG=
Review-Url: https://codereview.chromium.org/2595933002
Cr-Commit-Position: refs/heads/master@{#442885}
Committed: https://chromium.googlesource.com/chromium/src/+/8c49df5372a3ee28001a5102326201a857450522
Patch Set 1 #
Messages
Total messages: 11 (5 generated)
Description was changed from ========== Change assert in RecentTabsPage.destroy() BUG= ========== to ========== Change assert in RecentTabsPage.destroy(). It always failed when the RecentTabsPage was closed without opening a recent tab. BUG= ==========
finkm@chromium.org changed reviewers: + bauerb@chromium.org
PTAL - Does that make sense? It's a shot in the dark. But I noticed that the current assert always fails if the RecentTapPage is closed when the user didn't open a tab.
On 2017/01/10 16:47:51, finkm wrote: > PTAL - Does that make sense? It's a shot in the dark. But I noticed that the > current assert always fails if the RecentTapPage is closed when the user didn't > open a tab. Seems reasonable. Just for the sake of due diligence, can you post a stack for the failing assert?
On 2017/01/10 at 20:16:05, bauerb wrote: > On 2017/01/10 16:47:51, finkm wrote: > > PTAL - Does that make sense? It's a shot in the dark. But I noticed that the > > current assert always fails if the RecentTapPage is closed when the user didn't > > open a tab. > > Seems reasonable. Just for the sake of due diligence, can you post a stack for the failing assert? Sure. See below. Can you spot something? 01-11 09:46:18.904 7775 7775 E AndroidRuntime: java.lang.AssertionError: Destroy called before removed from window 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at org.chromium.chrome.browser.ntp.RecentTabsPage.destroy(RecentTabsPage.java:168) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at org.chromium.chrome.browser.tab.Tab.destroyNativePageInternal(Tab.java:2312) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at org.chromium.chrome.browser.tab.Tab.destroy(Tab.java:1978) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at org.chromium.chrome.browser.tabmodel.TabModelImpl.finalizeTabClosure(TabModelImpl.java:592) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at org.chromium.chrome.browser.tabmodel.TabModelImpl.commitTabClosure(TabModelImpl.java:321) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at org.chromium.chrome.browser.snackbar.undo.UndoBarController.commitTabClosure(UndoBarController.java:194) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at org.chromium.chrome.browser.snackbar.undo.UndoBarController.onDismissNoAction(UndoBarController.java:184) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at org.chromium.chrome.browser.snackbar.SnackbarCollection.removeCurrent(SnackbarCollection.java:49) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at org.chromium.chrome.browser.snackbar.SnackbarCollection.removeCurrentDueToTimeout(SnackbarCollection.java:72) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at org.chromium.chrome.browser.snackbar.SnackbarManager$1.run(SnackbarManager.java:74) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at android.os.Handler.handleCallback(Handler.java:751) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at android.os.Handler.dispatchMessage(Handler.java:95) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at android.os.Looper.loop(Looper.java:154) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at android.app.ActivityThread.main(ActivityThread.java:6119) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at java.lang.reflect.Method.invoke(Native Method) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886) 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)
On 2017/01/11 08:48:00, finkm wrote: > On 2017/01/10 at 20:16:05, bauerb wrote: > > On 2017/01/10 16:47:51, finkm wrote: > > > PTAL - Does that make sense? It's a shot in the dark. But I noticed that the > > > current assert always fails if the RecentTapPage is closed when the user > didn't > > > open a tab. > > > > Seems reasonable. Just for the sake of due diligence, can you post a stack for > the failing assert? > > Sure. See below. Can you spot something? > > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: java.lang.AssertionError: > Destroy called before removed from window > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > org.chromium.chrome.browser.ntp.RecentTabsPage.destroy(RecentTabsPage.java:168) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > org.chromium.chrome.browser.tab.Tab.destroyNativePageInternal(Tab.java:2312) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > org.chromium.chrome.browser.tab.Tab.destroy(Tab.java:1978) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > org.chromium.chrome.browser.tabmodel.TabModelImpl.finalizeTabClosure(TabModelImpl.java:592) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > org.chromium.chrome.browser.tabmodel.TabModelImpl.commitTabClosure(TabModelImpl.java:321) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > org.chromium.chrome.browser.snackbar.undo.UndoBarController.commitTabClosure(UndoBarController.java:194) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > org.chromium.chrome.browser.snackbar.undo.UndoBarController.onDismissNoAction(UndoBarController.java:184) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > org.chromium.chrome.browser.snackbar.SnackbarCollection.removeCurrent(SnackbarCollection.java:49) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > org.chromium.chrome.browser.snackbar.SnackbarCollection.removeCurrentDueToTimeout(SnackbarCollection.java:72) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > org.chromium.chrome.browser.snackbar.SnackbarManager$1.run(SnackbarManager.java:74) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > android.os.Handler.handleCallback(Handler.java:751) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > android.os.Handler.dispatchMessage(Handler.java:95) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > android.os.Looper.loop(Looper.java:154) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > android.app.ActivityThread.main(ActivityThread.java:6119) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > java.lang.reflect.Method.invoke(Native Method) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886) > 01-11 09:46:18.904 7775 7775 E AndroidRuntime: at > com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776) OK, thanks. I assume what happens here is that a parent view is detached from the hierarchy instead of this view, so the parent still exists. It also matches what we do in NewTabPage, so LGTM.
The CQ bit was checked by finkm@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1484136862434380, "parent_rev":
"3f999d3d054de11816d1a97c887753b26013c853", "commit_rev":
"8c49df5372a3ee28001a5102326201a857450522"}
Message was sent while issue was closed.
Description was changed from ========== Change assert in RecentTabsPage.destroy(). It always failed when the RecentTabsPage was closed without opening a recent tab. BUG= ========== to ========== Change assert in RecentTabsPage.destroy(). It always failed when the RecentTabsPage was closed without opening a recent tab. BUG= Review-Url: https://codereview.chromium.org/2595933002 Cr-Commit-Position: refs/heads/master@{#442885} Committed: https://chromium.googlesource.com/chromium/src/+/8c49df5372a3ee28001a51023262... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/8c49df5372a3ee28001a51023262... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
