Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(83)

Issue 372273004: Shutdown cleanup (Closed)

Created:
6 years, 5 months ago by Ben Goodger (Google)
Modified:
6 years, 5 months ago
Reviewers:
sky
CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org
Project:
chromium
Visibility:
Public.

Description

Attempt at cleaning up a bunch of shutdown issues. 1. Defers command buffer destruction until after the native viewport client has had a chance to cleanup. 2. Destroy the compositor before quitting the view manager message loop. 3. Make various NodeObservers properly unhook themselves as node/view observers during destruction and view swapping. 4. Adds a "ViewManagerDisconnected" method to ViewManagerDelegate to allow the application to exit when the connection to the view manager is closed. This is necessary in addition to root monitoring because for some connections (e.g. the root node->window manager) the root node is not destroyed. I'm not entirely happy with imposing this burden on application developers but I will try and tidy this up later. 5. Instantiate nested AtExitManagers only in a static build. In the component build we can use the shell's. 6. Adds a callback to notify the ViewManagerInitServiceImpl when the NativeViewport is destroyed. The VMISI uses this callback to close its connection to the view manager, which triggers the view manager to shut down. Since this still crashes, I can't test this yet. R=sky@chromium.org http://crbug.com/325901 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282683

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Patch Set 4 : . #

Patch Set 5 : . #

Patch Set 6 : . #

Patch Set 7 : . #

Patch Set 8 : . #

Patch Set 9 : . #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+155 lines, -77 lines) Patch
M mojo/examples/aura_demo/aura_demo.cc View 1 2 3 4 5 6 1 chunk +4 lines, -0 lines 0 comments Download
M mojo/examples/browser/browser.cc View 1 2 3 4 5 6 2 chunks +10 lines, -0 lines 0 comments Download
M mojo/examples/compositor_app/compositor_app.cc View 1 chunk +2 lines, -1 line 0 comments Download
M mojo/examples/embedded_app/embedded_app.cc View 1 2 3 4 5 6 2 chunks +4 lines, -0 lines 0 comments Download
M mojo/examples/html_viewer/html_viewer.cc View 1 2 3 4 5 6 2 chunks +5 lines, -0 lines 0 comments Download
M mojo/examples/keyboard/keyboard.cc View 1 2 3 4 5 6 1 chunk +6 lines, -0 lines 0 comments Download
M mojo/examples/media_viewer/media_viewer.cc View 1 2 3 4 5 6 1 chunk +6 lines, -0 lines 0 comments Download
M mojo/examples/nesting_app/nesting_app.cc View 1 2 3 4 5 6 2 chunks +4 lines, -0 lines 0 comments Download
M mojo/examples/pepper_container_app/pepper_container_app.cc View 2 chunks +2 lines, -1 line 0 comments Download
M mojo/examples/png_viewer/png_viewer.cc View 1 2 3 4 5 6 2 chunks +5 lines, -0 lines 0 comments Download
M mojo/examples/sample_app/sample_app.cc View 1 chunk +3 lines, -1 line 0 comments Download
M mojo/examples/window_manager/window_manager.cc View 1 2 3 4 5 6 3 chunks +8 lines, -6 lines 0 comments Download
M mojo/public/cpp/application/lib/mojo_main_chromium.cc View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M mojo/services/native_viewport/native_viewport_service.cc View 1 chunk +6 lines, -3 lines 0 comments Download
M mojo/services/network/main.cc View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M mojo/services/public/cpp/view_manager/lib/node.cc View 1 2 2 chunks +2 lines, -21 lines 0 comments Download
M mojo/services/public/cpp/view_manager/lib/view.cc View 1 2 2 chunks +3 lines, -23 lines 0 comments Download
M mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.cc View 1 2 3 4 5 6 7 2 chunks +2 lines, -0 lines 0 comments Download
M mojo/services/public/cpp/view_manager/tests/view_manager_unittest.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M mojo/services/public/cpp/view_manager/view_manager_delegate.h View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
M mojo/services/public/interfaces/native_viewport/native_viewport.mojom View 1 chunk +1 line, -1 line 1 comment Download
M mojo/services/view_manager/root_node_manager.h View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
M mojo/services/view_manager/root_node_manager.cc View 1 2 3 4 1 chunk +8 lines, -3 lines 0 comments Download
M mojo/services/view_manager/root_view_manager.h View 1 2 3 2 chunks +3 lines, -1 line 0 comments Download
M mojo/services/view_manager/root_view_manager.cc View 1 2 3 2 chunks +7 lines, -4 lines 0 comments Download
M mojo/services/view_manager/view_manager_init_service_impl.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M mojo/services/view_manager/view_manager_init_service_impl.cc View 1 2 3 4 5 6 7 8 3 chunks +13 lines, -1 line 0 comments Download
M mojo/services/view_manager/window_tree_host_impl.h View 1 2 3 3 chunks +8 lines, -5 lines 0 comments Download
M mojo/services/view_manager/window_tree_host_impl.cc View 1 2 3 4 5 2 chunks +8 lines, -3 lines 1 comment Download
M mojo/views/native_widget_view_manager.h View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M mojo/views/native_widget_view_manager.cc View 1 2 3 chunks +18 lines, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
Ben Goodger (Google)
6 years, 5 months ago (2014-07-10 22:26:59 UTC) #1
sky
LGTM https://codereview.chromium.org/372273004/diff/150001/mojo/services/public/interfaces/native_viewport/native_viewport.mojom File mojo/services/public/interfaces/native_viewport/native_viewport.mojom (right): https://codereview.chromium.org/372273004/diff/150001/mojo/services/public/interfaces/native_viewport/native_viewport.mojom#newcode24 mojo/services/public/interfaces/native_viewport/native_viewport.mojom:24: OnDestroyed() => (); Seems unusual to have a ...
6 years, 5 months ago (2014-07-10 22:54:30 UTC) #2
Ben Goodger (Google)
6 years, 5 months ago (2014-07-11 20:27:21 UTC) #3
Message was sent while issue was closed.
Committed patchset #9 manually as r282683 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698