Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2522093002:
Notify WidgetRemovalsObserver when RootView is removed (Closed)
|
Created:
4 years, 1 month ago by Sarmad Hashmi Modified:
4 years ago CC:
chromium-reviews, aboxhall+watch_chromium.org, tfarina, nektar+watch_chromium.org, yuzo+watch_chromium.org, je_julie, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionNotify WidgetRemovalsObserver when RootView is removed
RootViews are destroyed before OnWindowDestroying when widgets are initialized
with ownership set to Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET.
In this case, WidgetObservers will never be able to get a
reference to a RootView. This CL notifies WidgetRemovalsObservers
when a RootView is being removed using the existing WillRemoveView.
BUG=648701
Committed: https://crrev.com/cb3910d325c997dc3746667cde868ebf5435c3aa
Cr-Commit-Position: refs/heads/master@{#434167}
Patch Set 1 #Patch Set 2 : Notify WidgetRemovalsObserver when RootView is removed #Patch Set 3 : Add test #
Dependent Patchsets: Messages
Total messages: 27 (19 generated)
The CQ bit was checked by mhashmi@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...
Description was changed from ========== Notify WidgetRemovalsObserver when RootView is removed RootViews are destroyed before OnWindowDestroying so WidgetObservers will never be able to get a reference to a RootView. This CL notifies WidgetRemovalsObservers when a RootView is being removed. BUG=648701 ========== to ========== Notify WidgetRemovalsObserver when RootView is removed RootViews are destroyed before OnWindowDestroying when widgets are initialized with ownership set to Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET. In this case, WidgetObservers will never be able to get a reference to a RootView. This CL notifies WidgetRemovalsObservers when a RootView is being removed using the existing WillRemoveView. BUG=648701 ==========
Description was changed from ========== Notify WidgetRemovalsObserver when RootView is removed RootViews are destroyed before OnWindowDestroying when widgets are initialized with ownership set to Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET. In this case, WidgetObservers will never be able to get a reference to a RootView. This CL notifies WidgetRemovalsObservers when a RootView is being removed using the existing WillRemoveView. BUG=648701 ========== to ========== Notify WidgetRemovalsObserver when RootView is removed RootViews are destroyed before OnWindowDestroying when widgets are initialized with ownership set to Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET. In this case, WidgetObservers will never be able to get a reference to a RootView. This CL notifies WidgetRemovalsObservers when a RootView is being removed using the existing WillRemoveView. BUG=648701 ==========
Description was changed from ========== Notify WidgetRemovalsObserver when RootView is removed RootViews are destroyed before OnWindowDestroying when widgets are initialized with ownership set to Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET. In this case, WidgetObservers will never be able to get a reference to a RootView. This CL notifies WidgetRemovalsObservers when a RootView is being removed using the existing WillRemoveView. BUG=648701 ========== to ========== Notify WidgetRemovalsObserver when RootView is removed RootViews are destroyed before OnWindowDestroying when widgets are initialized with ownership set to Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET. In this case, WidgetObservers will never be able to get a reference to a RootView. This CL notifies WidgetRemovalsObservers when a RootView is being removed using the existing WillRemoveView. BUG=648701 ==========
The CQ bit was checked by mhashmi@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...
mhashmi@chromium.org changed reviewers: + sadrul@chromium.org, sky@chromium.org
As discussed with sadrul@, when Widget owns NativeWidget, OnWidgetDestroying and OnWindowDestroying (for the widget's window) are called after the RootView is destroyed. This causes a problem in DevTools when we try to destroy the subtree for the window + widget, the widget doesn't have a RootView at that point and so we never clear it from the view->nodeid map. By overriding OnWillRemoveView (after this CL), we can simply check if the view being removed is the root view and destroy the whole subtree.
LGTM
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Can you write a test?
The CQ bit was checked by mhashmi@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...
Added a test.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by mhashmi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org Link to the patchset: https://codereview.chromium.org/2522093002/#ps40001 (title: "Add test")
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": 40001, "attempt_start_ts": 1479913296769110,
"parent_rev": "2271bdf1d2784e8cddd5526b9787ff3452b67a44", "commit_rev":
"97f0718972fca41f534a0a8fe9add0151d126e2c"}
Message was sent while issue was closed.
Description was changed from ========== Notify WidgetRemovalsObserver when RootView is removed RootViews are destroyed before OnWindowDestroying when widgets are initialized with ownership set to Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET. In this case, WidgetObservers will never be able to get a reference to a RootView. This CL notifies WidgetRemovalsObservers when a RootView is being removed using the existing WillRemoveView. BUG=648701 ========== to ========== Notify WidgetRemovalsObserver when RootView is removed RootViews are destroyed before OnWindowDestroying when widgets are initialized with ownership set to Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET. In this case, WidgetObservers will never be able to get a reference to a RootView. This CL notifies WidgetRemovalsObservers when a RootView is being removed using the existing WillRemoveView. BUG=648701 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Notify WidgetRemovalsObserver when RootView is removed RootViews are destroyed before OnWindowDestroying when widgets are initialized with ownership set to Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET. In this case, WidgetObservers will never be able to get a reference to a RootView. This CL notifies WidgetRemovalsObservers when a RootView is being removed using the existing WillRemoveView. BUG=648701 ========== to ========== Notify WidgetRemovalsObserver when RootView is removed RootViews are destroyed before OnWindowDestroying when widgets are initialized with ownership set to Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET. In this case, WidgetObservers will never be able to get a reference to a RootView. This CL notifies WidgetRemovalsObservers when a RootView is being removed using the existing WillRemoveView. BUG=648701 Committed: https://crrev.com/cb3910d325c997dc3746667cde868ebf5435c3aa Cr-Commit-Position: refs/heads/master@{#434167} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/cb3910d325c997dc3746667cde868ebf5435c3aa Cr-Commit-Position: refs/heads/master@{#434167} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
