Mir

Merge lp:~robertcarr/mir/focus-tell-dont-ask into lp:mir

  1. focus-tell-dont-ask
  2. Merge into development-branch
Proposed by Robert Carr
Status: Rejected
Rejected by: Robert Carr
Proposed branch: lp:~robertcarr/mir/focus-tell-dont-ask
Merge into: lp:mir
Diff against target: 919 lines (+385/-113)
26 files modified
debian/changelog (+8/-2)
include/server/mir/default_server_configuration.h (+3/-0)
include/server/mir/frontend/connector_report.h (+0/-1)
include/server/mir/frontend/messenger_report.h (+49/-0)
include/server/mir/frontend/protobuf_ipc_factory.h (+3/-1)
include/server/mir/logging/messenger_report.h (+49/-0)
include/server/mir/shell/application_session.h (+6/-0)
include/server/mir/shell/default_focus_mechanism.h (+2/-2)
include/server/mir/shell/session.h (+5/-0)
include/test/mir_test_doubles/mock_shell_session.h (+3/-0)
include/test/mir_test_doubles/stub_ipc_factory.h (+6/-1)
include/test/mir_test_doubles/stub_shell_session.h (+8/-0)
src/server/default_server_configuration.cpp (+31/-3)
src/server/frontend/CMakeLists.txt (+1/-0)
src/server/frontend/null_messenger_report.cpp (+25/-0)
src/server/frontend/protobuf_session_creator.cpp (+2/-2)
src/server/frontend/socket_messenger.cpp (+11/-2)
src/server/frontend/socket_messenger.h (+6/-1)
src/server/logging/CMakeLists.txt (+1/-0)
src/server/logging/messenger_report.cpp (+46/-0)
src/server/shell/application_session.cpp (+39/-3)
src/server/shell/default_focus_mechanism.cpp (+10/-18)
tests/acceptance-tests/test_client_focus_notification.cpp (+3/-1)
tests/unit-tests/frontend/CMakeLists.txt (+1/-0)
tests/unit-tests/frontend/test_socket_messenger.cpp (+37/-0)
tests/unit-tests/shell/test_default_focus_mechanism.cpp (+30/-76)
To merge this branch: bzr merge lp:~robertcarr/mir/focus-tell-dont-ask
Reviewer Review Type Date Requested Status
Robert Carr (community) Disapprove
Alan Griffiths Needs Fixing
Daniel van Vugt Needs Fixing
PS Jenkins bot (community) continuous-integration Needs Fixing
Alexandros Frantzis Pending
kevin gunn Pending
Review via email: mp+188397@code.launchpad.net

This proposal supersedes a proposal from 2013-09-26.

Description of the change

Broken pipe isnt present in trunk anymore so resubmitting without prereq.

===

Fixed error in ApplicationSession::relinquish_focus :)

Targeting to development-branch

===

Solve some of the races around focus by applying 'tell-dont-ask' in relation to internal Session state and the Focus Mechanism. Now the focus mechanism doesn't have to hold an (unsafe) shared_ptr to the last focused surface. It's ok for the Session to hold this weak_ptr, as it only attempts to lock it when holding surfaces_mutex.

This branch requires: https://code.launchpad.net/~vanvugt/mir/fix-1226139/+merge/186809

As it will cause unfocus messages to be sent in the "connection dropped without disconnect case"(i.e. many of the protobuf tool tests which fail to call disconnect) very likely to lead to a broken pipe error.

== Old Description ==

Fix the races around focus. Essentially these stem from that holding std::shared_ptr<msh::Surface> doesn't guarantee that calling methods on that surface will be safe (the underlying surface could be destroyed via Surface::destroy and then your method call will throw).

So this branch changes ApplicationSession to not destroy the surface explicitly and instead rely on ~msh::Surface, which means that you really can hold a safe pointer. I think this as a reasonable choice, as the shell might want to do similar things, i.e. continue to animate a surface after a session has closed it.

==

New approach relies on ApplicationSession to hold the last focused ptr...WIP

To post a comment you must log in.
Revision history for this message
Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal

I was able to run the stress test with continuous cursor input for the full ten minute run. There is also good reason to believe this fixes: https://bugs.launchpad.net/mir/+bug/1220845

The exception in ~SessionMediator can be triggered if session_manager->close_session throws (due to this focus race), causing SessionMediator::disconnect to fail to reset it's session member-variable pointer. This exception is caught in frontend. However now in ~SessionMediator the pointer has not been reset so it closes the session again throwing an Invalid Session exception. causing the session to be closed again in ~

Revision history for this message
Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal

The unsafely held surface pointer in this case is the previously focused app.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:1054
http://jenkins.qa.ubuntu.com/job/mir-ci/1446/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1952
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1837
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/684
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/684/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/1446/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal

There is an inconsistency with this approach: when a session is closed its surface resources are destroyed immediately, but when a surface is explicitly destroyed the resources are not destroyed until there are no more users.

Would it (make sense|be possible) for msh::Surface to own (hold a strong pointer to) ms::Surface and rely on ~msh::Surface() in both the cases above, thus removing the msh::Surface -> ms::Surface weak_ptr dance?

review: Needs Information
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

> There is an inconsistency with this approach: when a session is closed its
> surface resources are destroyed immediately, but when a surface is explicitly
> destroyed the resources are not destroyed until there are no more users.
>
> Would it (make sense|be possible) for msh::Surface to own (hold a strong
> pointer to) ms::Surface and rely on ~msh::Surface() in both the cases above,
> thus removing the msh::Surface -> ms::Surface weak_ptr dance?

I've not yet looked at the MP, but I can contribute the origin of the "weak_ptr dance".

Once a surface has been "destroyed" it is no longer in the scenegraph (ok, SurfaceStack) and therefore not composited. If the ms::Surface continued to exist it could reach a state where it has client buffers meaning threads that require them will wait forever.

It may well be that subsequent changes (particularly the introduction of force_requests_to_complete()) permits a better solution than the "weak_ptr dance".

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

I don't think this is the right evolution of the code, but I don't think it makes things worse.

The right solution would be more complex and should probably come as part of fixing the data model.

review: Abstain
Revision history for this message
Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal

I think it makes sense to not destroy the surfaces explicitly from the session (as in the last committ). It may be possible to fix the weak_ptr dance (it almost surely is possible). It's not really clear what direction this code should go in right now though (except probably not further down this one), so I am trying to just fix the crashes non invasively

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:1055
http://jenkins.qa.ubuntu.com/job/mir-ci/1454/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1963
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1848
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/692
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/692/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/1454/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Bug 1216727 is still happening :(

terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<std::logic_error> >'
  what(): Requesting handle for an unregistered channel

Should we just unlink it from the branch?

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

P.S. Before declaring bug 1216727 fixed, please find a machine on which you can reproduce it (using trunk). To be sure the changes are actually having an effect.

Revision history for this message
Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal

Can someone review this again?

Revision history for this message
Daniel van Vugt (vanvugt) : Posted in a previous version of this proposal
review: Abstain
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1057
http://jenkins.qa.ubuntu.com/job/mir-ci/1524/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2062
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1947
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/762/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/38
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/38/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/1524/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Weirdly, merging this branch now causes an almost immediate hang:

Running tests...
Test project /home/dan/bzr/mir/tmp.focus/build
        Start 1: LGPL-required
  1/145 Test #1: LGPL-required ............................................... Passed 0.01 sec
        Start 2: GPL-required
  2/145 Test #2: GPL-required ................................................ Passed 0.01 sec
        Start 3: acceptance-tests.BespokeDisplayServerTestFixture.*

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1058
http://jenkins.qa.ubuntu.com/job/mir-ci/1528/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2068
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1953
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/766/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/42
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/42/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/1528/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Another bug with this branch is that on Alt+Tab in demo_server_shell, the client rendering freezes. Permanently.

review: Needs Fixing
Revision history for this message
Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal

The diff noise is because I merged https://code.launchpad.net/~robertcarr/mir/socket-messenger-reporting/+merge/187023 in but only set the branch dependency on https://code.launchpad.net/~vanvugt/mir/fix-1226139/+merge/186809

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1084
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~robertcarr/mir/focus-tell-dont-ask/+merge/187037/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-ci/1558/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2090
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1975
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/796/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/58
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/58/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/1558/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
kevin gunn (kgunn72) wrote : Posted in a previous version of this proposal

wouldn't this require a server api bump? i think so...

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1085
http://jenkins.qa.ubuntu.com/job/mir-ci/1562/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2098
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1983
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/800/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/62
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/62/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/1562/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

On both g++ and clang 64bit builds I see:

[ FAILED ] 2 tests, listed below:
[ FAILED ] BespokeDisplayServerTestFixture.a_surface_is_notified_of_receiving_focus
[ FAILED ] BespokeDisplayServerTestFixture.two_surfaces_are_notified_of_gaining_and_losing_focus

(and so does jenkins on amd64)

review: Needs Fixing
Revision history for this message
Robert Ancell (robert-ancell) wrote : Posted in a previous version of this proposal

This merge should be moved to lp:~mir-team/mir/development-branch

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1085
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~robertcarr/mir/focus-tell-dont-ask/+merge/187648/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/31/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2142/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2027/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/31/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/31/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1088
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~robertcarr/mir/focus-tell-dont-ask/+merge/187886/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/44/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2157
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2042
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/44
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/44/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/44/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:1088
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/45/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2159
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2044
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/45
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/45/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/45/rebuild

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote : Posted in a previous version of this proposal

I think the idea is a step in the right direction, given the problem :) (makes more sense than the shared_ptr in the focus controller)

receive_focus/relinquish focus seems a funny name for what's going on, given that there's no exchange of resources.
There's some gaps with who really has the-focus though still, like with,
148 + if (surfaces.size() == 0)
149 + {
150 + targeter->focus_cleared();
151 + return;
152 + }
when this happens, the focus controller is unaware that it could reassign the focus... perhaps the session should accept or reject the focus, and let the focus mechanism know. I Maybe we're twisting around the funny way ms::Surface is.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

It looked weird before, it looks weird after

review: Abstain
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

The prereq branch needs fixing or even rejecting. And therefore its dependents will need updating too.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1088
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~robertcarr/mir/focus-tell-dont-ask/+merge/188397/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/58/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2183
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2068
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/58
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/58/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/58/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The changes to debian/changelog are old and/or wrong. Even if there's a robot cleaning it up on lp:mir, the changelog entries are still not related to this proposal.

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I think the MessengerReport changes are probably spurious.

review: Needs Fixing
Revision history for this message
Robert Carr (robertcarr) wrote :

Not required if we can land hold-surface-alive

review: Disapprove

Unmerged revisions

1088. By Robert Carr

Fix error in relinquish focus

1087. By Robert Carr

Merge socket-messenger-reporitng

1086. By Robert Carr

Merge development-branch

1085. By Robert Carr

Typo

1084. By Robert Carr

application_session: Introduce default_surface_locked to dedupe some code

1083. By Robert Carr

Merge fix-1226139

1082. By Robert Carr

More cleanup

1081. By Robert Carr

Merge trunk

1080. By Robert Carr

Cleanup

1079. By Robert Carr

Mutex for surface

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2013-09-26 02:41:07 +0000
3+++ debian/changelog 2013-09-30 17:13:36 +0000
4@@ -1,4 +1,4 @@
5-mir (0.0.12-0ubuntu1) UNRELEASED; urgency=low
6+mir (0.0.12+13.10.20130926.1-0ubuntu1) saucy; urgency=low
7
8 [ kg ]
9 * bump version for ABI break (LP: #1229212)
10@@ -6,7 +6,13 @@
11 [ Robert Ancell ]
12 * Bump version to 0.0.12
13
14- -- Robert Ancell <robert.ancell@canonical.com> Thu, 26 Sep 2013 09:34:23 +1200
15+ [ Alexandros Frantzis ]
16+ * tests: Fix compiler warning about maybe-uninitialized struct member
17+
18+ [ Ubuntu daily release ]
19+ * Automatic snapshot from revision 1084
20+
21+ -- Ubuntu daily release <ps-jenkins@lists.canonical.com> Thu, 26 Sep 2013 08:39:29 +0000
22
23 mir (0.0.11+13.10.20130924.1-0ubuntu1) saucy; urgency=low
24
25
26=== modified file 'include/server/mir/default_server_configuration.h'
27--- include/server/mir/default_server_configuration.h 2013-09-24 11:43:27 +0000
28+++ include/server/mir/default_server_configuration.h 2013-09-30 17:13:36 +0000
29@@ -46,6 +46,7 @@
30 class SessionCreator;
31 class SessionMediatorReport;
32 class MessageProcessorReport;
33+class MessengerReport;
34 class SessionAuthorizer;
35 class EventSink;
36 class DisplayChanger;
37@@ -165,6 +166,7 @@
38 * @{ */
39 virtual std::shared_ptr<frontend::SessionMediatorReport> the_session_mediator_report();
40 virtual std::shared_ptr<frontend::MessageProcessorReport> the_message_processor_report();
41+ virtual std::shared_ptr<frontend::MessengerReport> the_messenger_report();
42 virtual std::shared_ptr<frontend::SessionAuthorizer> the_session_authorizer();
43 virtual std::shared_ptr<frontend::Shell> the_frontend_shell();
44 virtual std::shared_ptr<frontend::EventSink> the_global_event_sink();
45@@ -270,6 +272,7 @@
46 CachedPtr<frontend::ProtobufIpcFactory> ipc_factory;
47 CachedPtr<frontend::SessionMediatorReport> session_mediator_report;
48 CachedPtr<frontend::MessageProcessorReport> message_processor_report;
49+ CachedPtr<frontend::MessengerReport> messenger_report;
50 CachedPtr<frontend::SessionAuthorizer> session_authorizer;
51 CachedPtr<frontend::EventSink> global_event_sink;
52 CachedPtr<frontend::SessionCreator> session_creator;
53
54=== modified file 'include/server/mir/frontend/connector_report.h'
55--- include/server/mir/frontend/connector_report.h 2013-09-24 16:20:17 +0000
56+++ include/server/mir/frontend/connector_report.h 2013-09-30 17:13:36 +0000
57@@ -42,7 +42,6 @@
58 class NullConnectorReport : public ConnectorReport
59 {
60 public:
61-
62 void error(std::exception const& error);
63 };
64 }
65
66=== added file 'include/server/mir/frontend/messenger_report.h'
67--- include/server/mir/frontend/messenger_report.h 1970-01-01 00:00:00 +0000
68+++ include/server/mir/frontend/messenger_report.h 2013-09-30 17:13:36 +0000
69@@ -0,0 +1,49 @@
70+/*
71+ * Copyright © 2013 Canonical Ltd.
72+ *
73+ * This program is free software: you can redistribute it and/or modify it
74+ * under the terms of the GNU General Public License version 3,
75+ * as published by the Free Software Foundation.
76+ *
77+ * This program is distributed in the hope that it will be useful,
78+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
79+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
80+ * GNU General Public License for more details.
81+ *
82+ * You should have received a copy of the GNU General Public License
83+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
84+ *
85+ * Authored by: Robert Carr <robert.carr@canonical.com>
86+ */
87+
88+#ifndef MIR_FRONTEND_MESSENGER_REPORT_H_
89+#define MIR_FRONTEND_MESSENGER_REPORT_H_
90+
91+#include <string>
92+
93+namespace mir
94+{
95+namespace frontend
96+{
97+
98+class MessengerReport
99+{
100+public:
101+ virtual void error(std::string const& error_message) = 0;
102+
103+protected:
104+ virtual ~MessengerReport() = default;
105+ MessengerReport() = default;
106+ MessengerReport(const MessengerReport&) = delete;
107+ MessengerReport& operator=(const MessengerReport&) = delete;
108+};
109+
110+class NullMessengerReport : public MessengerReport
111+{
112+public:
113+ void error(std::string const& error_message);
114+};
115+}
116+}
117+
118+#endif // MIR_FRONTEND_MESSENGER_REPORT_H_
119
120=== modified file 'include/server/mir/frontend/protobuf_ipc_factory.h'
121--- include/server/mir/frontend/protobuf_ipc_factory.h 2013-08-28 03:41:48 +0000
122+++ include/server/mir/frontend/protobuf_ipc_factory.h 2013-09-30 17:13:36 +0000
123@@ -32,6 +32,7 @@
124 class EventSink;
125 class ResourceCache;
126 class MessageProcessorReport;
127+class MessengerReport;
128
129 class ProtobufIpcFactory
130 {
131@@ -39,7 +40,8 @@
132 virtual std::shared_ptr<protobuf::DisplayServer> make_ipc_server(
133 std::shared_ptr<EventSink> const& sink, bool authorized_to_resize_display) = 0;
134 virtual std::shared_ptr<ResourceCache> resource_cache() = 0;
135- virtual std::shared_ptr<MessageProcessorReport> report() = 0;
136+ virtual std::shared_ptr<MessageProcessorReport> message_processor_report() = 0;
137+ virtual std::shared_ptr<MessengerReport> messenger_report() = 0;
138
139 protected:
140 ProtobufIpcFactory() {}
141
142=== added file 'include/server/mir/logging/messenger_report.h'
143--- include/server/mir/logging/messenger_report.h 1970-01-01 00:00:00 +0000
144+++ include/server/mir/logging/messenger_report.h 2013-09-30 17:13:36 +0000
145@@ -0,0 +1,49 @@
146+/*
147+ * Copyright © 2013 Canonical Ltd.
148+ *
149+ * This program is free software: you can redistribute it and/or modify it
150+ * under the terms of the GNU General Public License version 3,
151+ * as published by the Free Software Foundation.
152+ *
153+ * This program is distributed in the hope that it will be useful,
154+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
155+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
156+ * GNU General Public License for more details.
157+ *
158+ * You should have received a copy of the GNU General Public License
159+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
160+ *
161+ * Authored by: Alan Griffiths <alan@octopull.co.uk>
162+ */
163+
164+#ifndef MIR_LOGGING_MESSENGER_REPORT_H_
165+#define MIR_LOGGING_MESSENGER_REPORT_H_
166+
167+#include "mir/frontend/messenger_report.h"
168+
169+#include <memory>
170+
171+namespace mir
172+{
173+namespace logging
174+{
175+class Logger;
176+
177+class MessengerReport : public frontend::MessengerReport
178+{
179+public:
180+ MessengerReport(std::shared_ptr<Logger> const& logger);
181+ virtual ~MessengerReport() noexcept(true) = default;
182+
183+ void error(std::string const& error_message);
184+
185+private:
186+ char const* component();
187+ std::shared_ptr<Logger> const logger;
188+};
189+
190+}
191+}
192+
193+
194+#endif /* MIR_LOGGING_MESSENGER_REPORT_H_ */
195
196=== modified file 'include/server/mir/shell/application_session.h'
197--- include/server/mir/shell/application_session.h 2013-08-28 03:41:48 +0000
198+++ include/server/mir/shell/application_session.h 2013-09-30 17:13:36 +0000
199@@ -67,6 +67,9 @@
200
201 void set_lifecycle_state(MirLifecycleState state);
202
203+ void receive_focus(std::shared_ptr<InputTargeter> const& targeter, std::shared_ptr<SurfaceController> const& controller);
204+ void relinquish_focus();
205+
206 protected:
207 ApplicationSession(ApplicationSession const&) = delete;
208 ApplicationSession& operator=(ApplicationSession const&) = delete;
209@@ -79,6 +82,7 @@
210 std::shared_ptr<frontend::EventSink> const event_sink;
211
212 frontend::SurfaceId next_id();
213+ std::shared_ptr<Surface> default_surface_locked(std::unique_lock<std::mutex> const& lock) const;
214
215 std::atomic<int> next_surface_id;
216
217@@ -86,6 +90,8 @@
218 Surfaces::const_iterator checked_find(frontend::SurfaceId id) const;
219 std::mutex mutable surfaces_mutex;
220 Surfaces surfaces;
221+
222+ std::weak_ptr<Surface> last_focused_surface;
223 };
224
225 }
226
227=== modified file 'include/server/mir/shell/default_focus_mechanism.h'
228--- include/server/mir/shell/default_focus_mechanism.h 2013-08-28 03:41:48 +0000
229+++ include/server/mir/shell/default_focus_mechanism.h 2013-09-30 17:13:36 +0000
230@@ -50,8 +50,8 @@
231 std::shared_ptr<InputTargeter> const input_targeter;
232 std::shared_ptr<SurfaceController> const surface_controller;
233
234- std::mutex surface_focus_lock;
235- std::weak_ptr<Surface> currently_focused_surface;
236+ std::mutex focus_lock;
237+ std::weak_ptr<Session> currently_focused_session;
238 };
239
240 }
241
242=== modified file 'include/server/mir/shell/session.h'
243--- include/server/mir/shell/session.h 2013-08-28 03:41:48 +0000
244+++ include/server/mir/shell/session.h 2013-09-30 17:13:36 +0000
245@@ -28,6 +28,8 @@
246 namespace shell
247 {
248 class Surface;
249+class InputTargeter;
250+class SurfaceController;
251
252 class Session : public frontend::Session
253 {
254@@ -37,6 +39,9 @@
255
256 virtual void take_snapshot(SnapshotCallback const& snapshot_taken) = 0;
257 virtual std::shared_ptr<Surface> default_surface() const = 0;
258+
259+ virtual void receive_focus(std::shared_ptr<InputTargeter> const& targeter, std::shared_ptr<SurfaceController> const& controller) = 0;
260+ virtual void relinquish_focus() = 0;
261 };
262
263 }
264
265=== modified file 'include/test/mir_test_doubles/mock_shell_session.h'
266--- include/test/mir_test_doubles/mock_shell_session.h 2013-08-28 03:41:48 +0000
267+++ include/test/mir_test_doubles/mock_shell_session.h 2013-09-30 17:13:36 +0000
268@@ -49,6 +49,9 @@
269
270 MOCK_METHOD1(send_display_config, void(graphics::DisplayConfiguration const&));
271 MOCK_METHOD3(configure_surface, int(frontend::SurfaceId, MirSurfaceAttrib, int));
272+
273+ MOCK_METHOD2(receive_focus, void(std::shared_ptr<shell::InputTargeter> const&, std::shared_ptr<shell::SurfaceController> const&));
274+ MOCK_METHOD0(relinquish_focus, void());
275 };
276
277 }
278
279=== modified file 'include/test/mir_test_doubles/stub_ipc_factory.h'
280--- include/test/mir_test_doubles/stub_ipc_factory.h 2013-08-28 03:41:48 +0000
281+++ include/test/mir_test_doubles/stub_ipc_factory.h 2013-09-30 17:13:36 +0000
282@@ -24,6 +24,7 @@
283 #include "mir/frontend/protobuf_ipc_factory.h"
284 #include "mir/frontend/resource_cache.h"
285 #include "mir/frontend/null_message_processor_report.h"
286+#include "mir/frontend/messenger_report.h"
287
288 namespace mir
289 {
290@@ -53,10 +54,14 @@
291 return cache;
292 }
293
294- virtual std::shared_ptr<frontend::MessageProcessorReport> report()
295+ virtual std::shared_ptr<frontend::MessageProcessorReport> message_processor_report()
296 {
297 return std::make_shared<frontend::NullMessageProcessorReport>();
298 }
299+ virtual std::shared_ptr<frontend::MessengerReport> messenger_report()
300+ {
301+ return std::make_shared<frontend::NullMessengerReport>();
302+ }
303
304 std::shared_ptr<protobuf::DisplayServer> server;
305 std::shared_ptr<frontend::ResourceCache> const cache;
306
307=== modified file 'include/test/mir_test_doubles/stub_shell_session.h'
308--- include/test/mir_test_doubles/stub_shell_session.h 2013-08-13 22:20:37 +0000
309+++ include/test/mir_test_doubles/stub_shell_session.h 2013-09-30 17:13:36 +0000
310@@ -71,6 +71,14 @@
311 {
312 return std::shared_ptr<shell::Surface>();
313 }
314+
315+ void receive_focus(std::shared_ptr<shell::InputTargeter> const&,
316+ std::shared_ptr<shell::SurfaceController> const&)
317+ {
318+ }
319+ void relinquish_focus()
320+ {
321+ }
322 };
323
324 }
325
326=== modified file 'src/server/default_server_configuration.cpp'
327--- src/server/default_server_configuration.cpp 2013-09-24 17:25:47 +0000
328+++ src/server/default_server_configuration.cpp 2013-09-30 17:13:36 +0000
329@@ -30,6 +30,7 @@
330 #include "mir/frontend/protobuf_ipc_factory.h"
331 #include "mir/frontend/session_mediator_report.h"
332 #include "mir/frontend/null_message_processor_report.h"
333+#include "mir/frontend/messenger_report.h"
334 #include "mir/frontend/session_mediator.h"
335 #include "mir/frontend/session_authorizer.h"
336 #include "mir/frontend/global_event_sender.h"
337@@ -75,6 +76,7 @@
338 #include "mir/logging/session_mediator_report.h"
339 #include "mir/logging/message_processor_report.h"
340 #include "mir/logging/display_report.h"
341+#include "mir/logging/messenger_report.h"
342 #include "mir/lttng/message_processor_report.h"
343 #include "mir/lttng/input_report.h"
344 #include "mir/shell/surface_source.h"
345@@ -113,13 +115,15 @@
346 explicit DefaultIpcFactory(
347 std::shared_ptr<mf::Shell> const& shell,
348 std::shared_ptr<mf::SessionMediatorReport> const& sm_report,
349- std::shared_ptr<mf::MessageProcessorReport> const& mr_report,
350+ std::shared_ptr<mf::MessageProcessorReport> const& mp_report,
351+ std::shared_ptr<mf::MessengerReport> const& messenger_report,
352 std::shared_ptr<mg::Platform> const& graphics_platform,
353 std::shared_ptr<mf::DisplayChanger> const& display_changer,
354 std::shared_ptr<mg::GraphicBufferAllocator> const& buffer_allocator) :
355 shell(shell),
356 sm_report(sm_report),
357- mp_report(mr_report),
358+ mp_report(mp_report),
359+ msger_report(messenger_report),
360 cache(std::make_shared<mf::ResourceCache>()),
361 graphics_platform(graphics_platform),
362 display_changer(display_changer),
363@@ -131,6 +135,7 @@
364 std::shared_ptr<mf::Shell> shell;
365 std::shared_ptr<mf::SessionMediatorReport> const sm_report;
366 std::shared_ptr<mf::MessageProcessorReport> const mp_report;
367+ std::shared_ptr<mf::MessengerReport> const msger_report;
368 std::shared_ptr<mf::ResourceCache> const cache;
369 std::shared_ptr<mg::Platform> const graphics_platform;
370 std::shared_ptr<mf::DisplayChanger> const display_changer;
371@@ -164,16 +169,21 @@
372 return cache;
373 }
374
375- virtual std::shared_ptr<mf::MessageProcessorReport> report()
376+ virtual std::shared_ptr<mf::MessageProcessorReport> message_processor_report()
377 {
378 return mp_report;
379 }
380+ virtual std::shared_ptr<mf::MessengerReport> messenger_report()
381+ {
382+ return msger_report;
383+ }
384 };
385
386 char const* const server_socket_opt = "file";
387 char const* const no_server_socket_opt = "no-file";
388 char const* const session_mediator_report_opt = "session-mediator-report";
389 char const* const msg_processor_report_opt = "msg-processor-report";
390+char const* const messenger_report_opt = "messenger-report";
391 char const* const display_report_opt = "display-report";
392 char const* const legacy_input_report_opt = "legacy-input-report";
393 char const* const input_report_opt = "input-report";
394@@ -824,6 +834,7 @@
395 shell,
396 the_session_mediator_report(),
397 the_message_processor_report(),
398+ the_messenger_report(),
399 the_graphics_platform(),
400 the_frontend_display_changer(), allocator);
401 });
402@@ -890,6 +901,23 @@
403 });
404 }
405
406+std::shared_ptr<mf::MessengerReport>
407+mir::DefaultServerConfiguration::the_messenger_report()
408+{
409+ return messenger_report(
410+ [this]() -> std::shared_ptr<mf::MessengerReport>
411+ {
412+ auto report_opt = the_options()->get(messenger_report_opt, off_opt_value);
413+ if (report_opt == log_opt_value)
414+ {
415+ return std::make_shared<ml::MessengerReport>(the_logger());
416+ }
417+ else
418+ {
419+ return std::make_shared<mf::NullMessengerReport>();
420+ }
421+ });
422+}
423
424 std::shared_ptr<ml::Logger> mir::DefaultServerConfiguration::the_logger()
425 {
426
427=== modified file 'src/server/frontend/CMakeLists.txt'
428--- src/server/frontend/CMakeLists.txt 2013-09-24 13:27:33 +0000
429+++ src/server/frontend/CMakeLists.txt 2013-09-30 17:13:36 +0000
430@@ -5,6 +5,7 @@
431 session_mediator.cpp
432 null_session_mediator_report.cpp
433 null_message_processor_report.cpp
434+ null_messenger_report.cpp
435 protobuf_message_processor.cpp
436 protobuf_buffer_packer.cpp
437 null_message_processor.cpp
438
439=== added file 'src/server/frontend/null_messenger_report.cpp'
440--- src/server/frontend/null_messenger_report.cpp 1970-01-01 00:00:00 +0000
441+++ src/server/frontend/null_messenger_report.cpp 2013-09-30 17:13:36 +0000
442@@ -0,0 +1,25 @@
443+/*
444+ * Copyright © 2013 Canonical Ltd.
445+ *
446+ * This program is free software: you can redistribute it and/or modify it
447+ * under the terms of the GNU General Public License version 3,
448+ * as published by the Free Software Foundation.
449+ *
450+ * This program is distributed in the hope that it will be useful,
451+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
452+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
453+ * GNU General Public License for more details.
454+ *
455+ * You should have received a copy of the GNU General Public License
456+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
457+ *
458+ * Authored by: Robert Carr <robert.carr@canonical.com>
459+ */
460+
461+#include "mir/frontend/messenger_report.h"
462+
463+namespace mf = mir::frontend;
464+
465+void mf::NullMessengerReport::error(std::string const&)
466+{
467+}
468
469=== modified file 'src/server/frontend/protobuf_session_creator.cpp'
470--- src/server/frontend/protobuf_session_creator.cpp 2013-09-24 13:27:33 +0000
471+++ src/server/frontend/protobuf_session_creator.cpp 2013-09-30 17:13:36 +0000
472@@ -53,7 +53,7 @@
473
474 void mf::ProtobufSessionCreator::create_session_for(std::shared_ptr<ba::local::stream_protocol::socket> const& socket)
475 {
476- auto const messenger = std::make_shared<detail::SocketMessenger>(socket);
477+ auto const messenger = std::make_shared<detail::SocketMessenger>(socket, ipc_factory->messenger_report());
478 auto const client_pid = messenger->client_pid();
479
480 if (session_authorizer->connection_is_allowed(client_pid))
481@@ -64,7 +64,7 @@
482 messenger,
483 ipc_factory->make_ipc_server(event_sink, authorized_to_resize_display),
484 ipc_factory->resource_cache(),
485- ipc_factory->report());
486+ ipc_factory->message_processor_report());
487
488 const auto& session = std::make_shared<mfd::SocketSession>(messenger, next_id(), connected_sessions, msg_processor);
489 connected_sessions->add(session);
490
491=== modified file 'src/server/frontend/socket_messenger.cpp'
492--- src/server/frontend/socket_messenger.cpp 2013-09-25 14:18:18 +0000
493+++ src/server/frontend/socket_messenger.cpp 2013-09-30 17:13:36 +0000
494@@ -18,13 +18,16 @@
495
496 #include "socket_messenger.h"
497 #include "mir/frontend/client_constants.h"
498+#include "mir/frontend/messenger_report.h"
499
500 namespace mfd = mir::frontend::detail;
501 namespace bs = boost::system;
502 namespace ba = boost::asio;
503
504-mfd::SocketMessenger::SocketMessenger(std::shared_ptr<ba::local::stream_protocol::socket> const& socket)
505- : socket(socket)
506+mfd::SocketMessenger::SocketMessenger(std::shared_ptr<ba::local::stream_protocol::socket> const& socket,
507+ std::shared_ptr<MessengerReport> const& report)
508+ : socket(socket),
509+ report(report)
510 {
511 whole_message.reserve(serialization_buffer_size);
512 }
513@@ -59,8 +62,14 @@
514 // function has completed (if it would be executed asynchronously.
515 // NOTE: we rely on this synchronous behavior as per the comment in
516 // mf::SessionMediator::create_surface
517+
518 boost::system::error_code err;
519 ba::write(*socket, ba::buffer(whole_message), err);
520+ if (!err)
521+ {
522+ report->error(err.message());
523+ }
524+
525 }
526
527 void mfd::SocketMessenger::send_fds(std::vector<int32_t> const& fds)
528
529=== modified file 'src/server/frontend/socket_messenger.h'
530--- src/server/frontend/socket_messenger.h 2013-08-28 03:41:48 +0000
531+++ src/server/frontend/socket_messenger.h 2013-09-30 17:13:36 +0000
532@@ -26,13 +26,16 @@
533 {
534 namespace frontend
535 {
536+class MessengerReport;
537+
538 namespace detail
539 {
540 class SocketMessenger : public MessageSender,
541 public MessageReceiver
542 {
543 public:
544- SocketMessenger(std::shared_ptr<boost::asio::local::stream_protocol::socket> const& socket);
545+ SocketMessenger(std::shared_ptr<boost::asio::local::stream_protocol::socket> const& socket,
546+ std::shared_ptr<frontend::MessengerReport> const& report);
547
548 void send(std::string const& body);
549 void send_fds(std::vector<int32_t> const& fds);
550@@ -42,6 +45,8 @@
551
552 private:
553 std::shared_ptr<boost::asio::local::stream_protocol::socket> socket;
554+ std::shared_ptr<MessengerReport> const report;
555+
556 std::vector<char> whole_message;
557 };
558 }
559
560=== modified file 'src/server/logging/CMakeLists.txt'
561--- src/server/logging/CMakeLists.txt 2013-08-28 03:41:48 +0000
562+++ src/server/logging/CMakeLists.txt 2013-09-30 17:13:36 +0000
563@@ -6,6 +6,7 @@
564 message_processor_report.cpp
565 display_report.cpp
566 input_report.cpp
567+ messenger_report.cpp
568 )
569
570 add_library(
571
572=== added file 'src/server/logging/messenger_report.cpp'
573--- src/server/logging/messenger_report.cpp 1970-01-01 00:00:00 +0000
574+++ src/server/logging/messenger_report.cpp 2013-09-30 17:13:36 +0000
575@@ -0,0 +1,46 @@
576+/*
577+ * Copyright © 2013 Canonical Ltd.
578+ *
579+ * This program is free software: you can redistribute it and/or modify it
580+ * under the terms of the GNU General Public License version 3,
581+ * as published by the Free Software Foundation.
582+ *
583+ * This program is distributed in the hope that it will be useful,
584+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
585+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
586+ * GNU General Public License for more details.
587+ *
588+ * You should have received a copy of the GNU General Public License
589+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
590+ *
591+ * Authored by: Alan Griffiths <alan@octopull.co.uk>
592+ */
593+
594+#include "mir/logging/messenger_report.h"
595+#include "mir/logging/logger.h"
596+
597+#include "std/MirLog.h"
598+#include <std/Log.h>
599+
600+
601+#include <sstream>
602+#include <cstring>
603+#include <mutex>
604+
605+namespace ml = mir::logging;
606+
607+ml::MessengerReport::MessengerReport(const std::shared_ptr<Logger>& logger)
608+ : logger(logger)
609+{
610+}
611+
612+const char* ml::MessengerReport::component()
613+{
614+ static const char* s = "messenger";
615+ return s;
616+}
617+
618+void ml::MessengerReport::error(std::string const& error_message)
619+{
620+ logger->log<Logger::informational>(error_message, component());
621+}
622
623=== modified file 'src/server/shell/application_session.cpp'
624--- src/server/shell/application_session.cpp 2013-08-28 03:41:48 +0000
625+++ src/server/shell/application_session.cpp 2013-09-30 17:13:36 +0000
626@@ -21,6 +21,7 @@
627 #include "mir/shell/surface_factory.h"
628 #include "mir/shell/snapshot_strategy.h"
629 #include "mir/shell/session_listener.h"
630+#include "mir/shell/input_targeter.h"
631 #include "mir/frontend/event_sink.h"
632
633 #include <boost/throw_exception.hpp>
634@@ -99,16 +100,21 @@
635 snapshot_strategy->take_snapshot_of(default_surface(), snapshot_taken);
636 }
637
638-std::shared_ptr<msh::Surface> msh::ApplicationSession::default_surface() const
639+std::shared_ptr<msh::Surface> msh::ApplicationSession::default_surface_locked(std::unique_lock<std::mutex> const& /* lock */) const
640 {
641- std::unique_lock<std::mutex> lock(surfaces_mutex);
642-
643 if (surfaces.size())
644 return surfaces.begin()->second;
645 else
646 return std::shared_ptr<msh::Surface>();
647 }
648
649+std::shared_ptr<msh::Surface> msh::ApplicationSession::default_surface() const
650+{
651+ std::unique_lock<std::mutex> lock(surfaces_mutex);
652+
653+ return default_surface_locked(lock);
654+}
655+
656 void msh::ApplicationSession::destroy_surface(mf::SurfaceId id)
657 {
658 std::unique_lock<std::mutex> lock(surfaces_mutex);
659@@ -171,3 +177,33 @@
660 {
661 event_sink->handle_lifecycle_event(state);
662 }
663+
664+void msh::ApplicationSession::receive_focus(std::shared_ptr<msh::InputTargeter> const& targeter,
665+ std::shared_ptr<SurfaceController> const& controller)
666+{
667+ std::unique_lock<std::mutex> lock(surfaces_mutex);
668+ if (surfaces.size() == 0)
669+ {
670+ targeter->focus_cleared();
671+ return;
672+ }
673+ // Use default_surface_locked idiom instead
674+ auto focus_surface = surfaces.begin()->second;
675+
676+ focus_surface->raise(controller);
677+ focus_surface->take_input_focus(targeter);
678+ focus_surface->configure(mir_surface_attrib_focus, mir_surface_focused);
679+
680+ last_focused_surface = focus_surface;
681+}
682+
683+void msh::ApplicationSession::relinquish_focus()
684+{
685+ std::unique_lock<std::mutex> lock(surfaces_mutex);
686+ auto surf = last_focused_surface.lock();
687+ if (surf)
688+ {
689+ surf->configure(mir_surface_attrib_focus, mir_surface_unfocused);
690+ }
691+ last_focused_surface.reset();
692+}
693
694=== modified file 'src/server/shell/default_focus_mechanism.cpp'
695--- src/server/shell/default_focus_mechanism.cpp 2013-08-28 03:41:48 +0000
696+++ src/server/shell/default_focus_mechanism.cpp 2013-09-30 17:13:36 +0000
697@@ -35,28 +35,20 @@
698
699 void msh::DefaultFocusMechanism::set_focus_to(std::shared_ptr<Session> const& focus_session)
700 {
701+ std::lock_guard<std::mutex> lg(focus_lock);
702+
703+ auto last_session = currently_focused_session.lock();
704+ if (last_session)
705+ {
706+ last_session->relinquish_focus();
707+ }
708+
709 // TODO: This path should be encapsulated in a seperate clear_focus message
710 if (!focus_session)
711 {
712 input_targeter->focus_cleared();
713 return;
714 }
715-
716- auto surface = focus_session->default_surface();
717- if (surface)
718- {
719- std::lock_guard<std::mutex> lg(surface_focus_lock);
720- auto current_focus = currently_focused_surface.lock();
721- if (current_focus)
722- current_focus->configure(mir_surface_attrib_focus, mir_surface_unfocused);
723- surface->configure(mir_surface_attrib_focus, mir_surface_focused);
724- currently_focused_surface = surface;
725-
726- surface->raise(surface_controller);
727- surface->take_input_focus(input_targeter);
728- }
729- else
730- {
731- input_targeter->focus_cleared();
732- }
733+ focus_session->receive_focus(input_targeter, surface_controller);
734+ currently_focused_session = focus_session;
735 }
736
737=== modified file 'tests/acceptance-tests/test_client_focus_notification.cpp'
738--- tests/acceptance-tests/test_client_focus_notification.cpp 2013-09-25 18:39:57 +0000
739+++ tests/acceptance-tests/test_client_focus_notification.cpp 2013-09-30 17:13:36 +0000
740@@ -152,7 +152,9 @@
741 void expect_events(mt::WaitCondition* all_events_received) override
742 {
743 EXPECT_CALL(*observer, see(Pointee(mt::SurfaceEvent(mir_surface_attrib_focus, mir_surface_focused)))).Times(1)
744- .WillOnce(mt::WakeUp(all_events_received));
745+ .WillOnce(mt::WakeUp(all_events_received)); // Now we close the surface generating an unfocused event
746+ // libmirclient guarantees we will see this before mir_surface_release finishes, so we don't need further
747+ // synchronization
748 EXPECT_CALL(*observer, see(Pointee(mt::SurfaceEvent(mir_surface_attrib_focus, mir_surface_unfocused)))).Times(1);
749 }
750 } client_config;
751
752=== modified file 'tests/unit-tests/frontend/CMakeLists.txt'
753--- tests/unit-tests/frontend/CMakeLists.txt 2013-09-24 14:13:15 +0000
754+++ tests/unit-tests/frontend/CMakeLists.txt 2013-09-30 17:13:36 +0000
755@@ -8,6 +8,7 @@
756 ${CMAKE_CURRENT_SOURCE_DIR}/test_resource_cache.cpp
757 ${CMAKE_CURRENT_SOURCE_DIR}/test_session_mediator.cpp
758 ${CMAKE_CURRENT_SOURCE_DIR}/test_socket_session.cpp
759+ ${CMAKE_CURRENT_SOURCE_DIR}/test_socket_messenger.cpp
760 ${CMAKE_CURRENT_SOURCE_DIR}/test_event_sender.cpp
761 ${CMAKE_CURRENT_SOURCE_DIR}/test_global_event_sender.cpp
762 )
763
764=== added file 'tests/unit-tests/frontend/test_socket_messenger.cpp'
765--- tests/unit-tests/frontend/test_socket_messenger.cpp 1970-01-01 00:00:00 +0000
766+++ tests/unit-tests/frontend/test_socket_messenger.cpp 2013-09-30 17:13:36 +0000
767@@ -0,0 +1,37 @@
768+/*
769+ * Copyright © 2013 Canonical Ltd.
770+ *
771+ * This program is free software: you can redistribute it and/or modify
772+ * it under the terms of the GNU General Public License version 3 as
773+ * published by the Free Software Foundation.
774+ *
775+ * This program is distributed in the hope that it will be useful,
776+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
777+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
778+ * GNU General Public License for more details.
779+ *
780+ * You should have received a copy of the GNU General Public License
781+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
782+ *
783+ * Authored by: Daniel van Vugt <daniel.van.vugt@canonical.com>
784+ */
785+
786+#include "mir/frontend/messenger_report.h"
787+#include "src/server/frontend/socket_messenger.h"
788+
789+#include <gmock/gmock.h>
790+#include <gtest/gtest.h>
791+
792+namespace mf = mir::frontend;
793+
794+using namespace mf::detail;
795+using namespace boost::asio;
796+
797+TEST(SocketMessengerTest, write_failures_never_throw)
798+{
799+ io_service svc;
800+ auto sock = std::make_shared<local::stream_protocol::socket>(svc);
801+ SocketMessenger mess(sock, std::make_shared<mf::NullMessengerReport>());
802+
803+ EXPECT_NO_THROW( mess.send("foo") );
804+}
805
806=== modified file 'tests/unit-tests/shell/test_default_focus_mechanism.cpp'
807--- tests/unit-tests/shell/test_default_focus_mechanism.cpp 2013-08-28 03:41:48 +0000
808+++ tests/unit-tests/shell/test_default_focus_mechanism.cpp 2013-09-30 17:13:36 +0000
809@@ -47,80 +47,34 @@
810 namespace mt = mir::test;
811 namespace mtd = mir::test::doubles;
812
813-TEST(DefaultFocusMechanism, raises_default_surface)
814-{
815- using namespace ::testing;
816-
817- NiceMock<mtd::MockShellSession> app1;
818- mtd::MockSurface mock_surface(&app1, std::make_shared<mtd::StubSurfaceBuilder>());
819- {
820- InSequence seq;
821- EXPECT_CALL(app1, default_surface()).Times(1)
822- .WillOnce(Return(mt::fake_shared(mock_surface)));
823- }
824-
825- auto controller = std::make_shared<mtd::StubSurfaceController>();
826- EXPECT_CALL(mock_surface, raise(Eq(controller))).Times(1);
827- mtd::StubInputTargeter targeter;
828- msh::DefaultFocusMechanism focus_mechanism(mt::fake_shared(targeter), controller);
829-
830- focus_mechanism.set_focus_to(mt::fake_shared(app1));
831-}
832-
833-TEST(DefaultFocusMechanism, mechanism_notifies_default_surface_of_focus_changes)
834-{
835- using namespace ::testing;
836-
837- NiceMock<mtd::MockShellSession> app1, app2;
838- mtd::MockSurface mock_surface1(&app1, std::make_shared<mtd::StubSurfaceBuilder>());
839- mtd::MockSurface mock_surface2(&app2, std::make_shared<mtd::StubSurfaceBuilder>());
840-
841- ON_CALL(app1, default_surface()).WillByDefault(Return(mt::fake_shared(mock_surface1)));
842- ON_CALL(app2, default_surface()).WillByDefault(Return(mt::fake_shared(mock_surface2)));
843-
844-
845- {
846- InSequence seq;
847- EXPECT_CALL(mock_surface1, configure(mir_surface_attrib_focus, mir_surface_focused)).Times(1);
848- EXPECT_CALL(mock_surface1, configure(mir_surface_attrib_focus, mir_surface_unfocused)).Times(1);
849- EXPECT_CALL(mock_surface2, configure(mir_surface_attrib_focus, mir_surface_focused)).Times(1);
850- }
851-
852- msh::DefaultFocusMechanism focus_mechanism(std::make_shared<mtd::StubInputTargeter>(),
853- std::make_shared<mtd::StubSurfaceController>());
854-
855- focus_mechanism.set_focus_to(mt::fake_shared(app1));
856- focus_mechanism.set_focus_to(mt::fake_shared(app2));
857-}
858-
859-TEST(DefaultFocusMechanism, sets_input_focus)
860-{
861- using namespace ::testing;
862-
863- NiceMock<mtd::MockShellSession> app1;
864- mtd::MockSurface mock_surface(&app1, std::make_shared<mtd::StubSurfaceBuilder>());
865- {
866- InSequence seq;
867- EXPECT_CALL(app1, default_surface()).Times(1)
868- .WillOnce(Return(mt::fake_shared(mock_surface)));
869- EXPECT_CALL(app1, default_surface()).Times(1)
870- .WillOnce(Return(std::shared_ptr<msh::Surface>()));
871- }
872-
873- mtd::MockInputTargeter targeter;
874-
875- msh::DefaultFocusMechanism focus_mechanism(mt::fake_shared(targeter), std::make_shared<mtd::StubSurfaceController>());
876-
877- {
878- InSequence seq;
879- EXPECT_CALL(mock_surface, take_input_focus(_)).Times(1);
880- // When we have no default surface.
881- EXPECT_CALL(targeter, focus_cleared()).Times(1);
882- // When we have no session.
883- EXPECT_CALL(targeter, focus_cleared()).Times(1);
884- }
885-
886- focus_mechanism.set_focus_to(mt::fake_shared(app1));
887- focus_mechanism.set_focus_to(mt::fake_shared(app1));
888- focus_mechanism.set_focus_to(std::shared_ptr<msh::Session>());
889+TEST(DefaultFocusMechanism, hands_focus_to_session)
890+{
891+ using namespace ::testing;
892+
893+ mtd::MockShellSession app1;
894+ EXPECT_CALL(app1, receive_focus(_, _)).Times(1);
895+
896+ msh::DefaultFocusMechanism focus_mechanism(std::make_shared<mtd::StubInputTargeter>(), std::make_shared<mtd::StubSurfaceController>());
897+
898+ focus_mechanism.set_focus_to(mt::fake_shared(app1));
899+}
900+
901+TEST(DefaultFocusMechanism, relinquishes_focus_from_last_session)
902+{
903+ using namespace ::testing;
904+
905+ auto app1 = std::make_shared<mtd::MockShellSession>();
906+ auto app2 = std::make_shared<mtd::MockShellSession>();
907+
908+ {
909+ InSequence seq;
910+ EXPECT_CALL(*app1, receive_focus(_, _)).Times(1);
911+ EXPECT_CALL(*app1, relinquish_focus()).Times(1);
912+ EXPECT_CALL(*app2, receive_focus(_, _)).Times(1);
913+ }
914+
915+ msh::DefaultFocusMechanism focus_mechanism(std::make_shared<mtd::StubInputTargeter>(), std::make_shared<mtd::StubSurfaceController>());
916+
917+ focus_mechanism.set_focus_to(app1);
918+ focus_mechanism.set_focus_to(app2);
919 }

Subscribers

People subscribed via source and target branches