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

Issue 474883003: Move focus from the view manager to the window manager. (Closed)

Created:
6 years, 4 months ago by Ben Goodger (Google)
Modified:
6 years, 4 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

Move focus from the view manager to the window manager. Focus and activation are closely related. Focus could perhaps be considered a view manager concept, but activation is window-manager specific, and cannot. Event dispatch is influenced by (among other things) focus. So, move event dispatch to the window manager. I still like the idea of the view manager client lib providing a "flattened" API to users that allows setting/querying focus state & observing changes, rather than obliging everyone connect to the window manager independently (though they may want to do so for other reasons). So I'm having the view manager client connect to the window manager & continue to provide the SetFocus()/FocusChanged APIs. This causes a minor dilemma in that we have two window managers but only one view manager client. Right now I resolved this by defaulting to the original window manager and allowing others to be specified on the command line to the shell, but I think we will eventually want a way to register a default window manager. R=sky@chromium.org BUG= Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289975

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 : . #

Patch Set 10 : . #

Patch Set 11 : . #

Total comments: 3

Patch Set 12 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+295 lines, -266 lines) Patch
M mojo/examples/window_manager/DEPS View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M mojo/examples/window_manager/window_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 7 chunks +23 lines, -13 lines 0 comments Download
M mojo/examples/wm_flow/wm/wm.cc View 1 2 3 4 5 3 chunks +2 lines, -6 lines 0 comments Download
M mojo/mojo_examples.gypi View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M mojo/mojo_services.gypi View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -0 lines 0 comments Download
M mojo/services/public/cpp/view_manager/BUILD.gn View 1 1 chunk +1 line, -0 lines 0 comments Download
M mojo/services/public/cpp/view_manager/lib/view_manager_client_factory.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.h View 1 2 3 4 5 4 chunks +17 lines, -4 lines 0 comments Download
M mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.cc View 1 2 3 4 5 6 chunks +41 lines, -18 lines 0 comments Download
M mojo/services/public/cpp/view_manager/view_manager.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M mojo/services/public/cpp/view_manager/window_manager_delegate.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M mojo/services/public/interfaces/view_manager/view_manager.mojom View 1 2 3 chunks +2 lines, -14 lines 0 comments Download
M mojo/services/view_manager/access_policy.h View 1 chunk +0 lines, -1 line 0 comments Download
M mojo/services/view_manager/default_access_policy.h View 1 chunk +0 lines, -1 line 0 comments Download
M mojo/services/view_manager/default_access_policy.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M mojo/services/view_manager/node.h View 1 2 1 chunk +0 lines, -3 lines 0 comments Download
M mojo/services/view_manager/node.cc View 1 2 1 chunk +0 lines, -4 lines 0 comments Download
M mojo/services/view_manager/node_delegate.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +0 lines, -8 lines 0 comments Download
M mojo/services/view_manager/root_node_manager.h View 1 2 5 chunks +2 lines, -12 lines 0 comments Download
M mojo/services/view_manager/root_node_manager.cc View 1 2 5 chunks +2 lines, -26 lines 0 comments Download
M mojo/services/view_manager/root_view_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +14 lines, -60 lines 0 comments Download
M mojo/services/view_manager/view_manager_service_impl.h View 1 2 chunks +0 lines, -5 lines 0 comments Download
M mojo/services/view_manager/view_manager_service_impl.cc View 1 2 chunks +0 lines, -29 lines 0 comments Download
M mojo/services/view_manager/view_manager_unittest.cc View 1 2 2 chunks +1 line, -9 lines 0 comments Download
M mojo/services/view_manager/window_manager_access_policy.h View 1 chunk +0 lines, -1 line 0 comments Download
M mojo/services/view_manager/window_manager_access_policy.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M mojo/services/view_manager/window_tree_host_impl.h View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M mojo/services/view_manager/window_tree_host_impl.cc View 1 2 2 chunks +4 lines, -5 lines 0 comments Download
M mojo/services/window_manager/DEPS View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M mojo/services/window_manager/main.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M mojo/services/window_manager/window_manager_app.h View 1 2 3 4 5 6 6 chunks +46 lines, -5 lines 0 comments Download
M mojo/services/window_manager/window_manager_app.cc View 1 2 3 4 5 6 7 8 9 10 11 10 chunks +126 lines, -26 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Ben Goodger (Google)
6 years, 4 months ago (2014-08-15 05:07:39 UTC) #1
sky
LGTM https://codereview.chromium.org/474883003/diff/200001/mojo/examples/window_manager/window_manager.cc File mojo/examples/window_manager/window_manager.cc (right): https://codereview.chromium.org/474883003/diff/200001/mojo/examples/window_manager/window_manager.cc#newcode366 mojo/examples/window_manager/window_manager.cc:366: window_manager_app_->host()->window()->AddPreTargetHandler(this); Do you need to remove the pre ...
6 years, 4 months ago (2014-08-15 18:03:56 UTC) #2
Ben Goodger (Google)
6 years, 4 months ago (2014-08-15 19:28:16 UTC) #3
Message was sent while issue was closed.
Committed patchset #12 manually as 289975 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698