Merge lp:~robertcarr/mir/disable-sequences-and-add-terminate-handler into lp:~mir-team/mir/trunk

[Robert Carr]

Disable control sequences (ctrl+c, etc...) and add a way to kill mir (demo shell) in lieu of this.

Disable control sequences (ctrl+c, etc...) and add a way to kill mir (demo shell) in lieu of this.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:692
http://jenkins.qa.ubuntu.com/job/mir-ci/589/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/696
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/579
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/73
        deb: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/73/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/318

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

review: Approve (continuous-integration)

[Robert Ancell (robert-ancell)]

The name disable_control_sequences() seems confusing - this is just setting the console into raw mode right? Shouldn't it be called set_raw_mode() or similar?

Is there an easy way for the demo shell to note that it can only be quit using alt+ctrl+backspace? This is likely to confuse people using this.

This change is going to affect all shells, not just the demo shell. It might be worth putting alt+ctrl+backspace into libmirserver by default (at least for now) so other shells also are quittable if we disable input.

Finally, check with Alexandros but I think there was some issues that meant he hadn't enabled raw input yet.

[Alexandros Frantzis (afrantzis)]

> This change is going to affect all shells, not just the demo shell.
> It might be worth putting alt+ctrl+backspace into libmirserver by default
> (at least for now) so other shells also are quittable if we disable input.

+1, we can put it in run_mir.cpp

> Finally, check with Alexandros but I think there was some issues that meant
> he hadn't enabled raw input yet.

My only concern is that we would disable VT switching key sequences, while we didn't yet have another way to handle them. It seems that the current raw mode options don't affect this, so no problem there.

Plus, I think that the input stack has matured enough to support this (i.e. catching ctrl-alt-f1 and initiating a VT switch), and when we get this we can completely disable VT input (something like ioctl(fd, KDSKBMODE, K_OFF), but I need to experiment), which means that we won't need FOPS::make_raw()/VT::disable_control_sequences() any more, we will just need VT::disable_input().

137 +void mgg::LinuxVirtualTerminal::disable_control_sequences()

We also need to restore the VT input handling state in the LinuxVirtualTerminal destructor, to get back to a sane console after shutdown.

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

In addition to Alexandros's comments around VT switching there should be some *documentation* to alert users that:

1. Ctrl-C doesn't kill mir_demo_shell
2. How to kill it

review: Needs Fixing

Unmerged revisions

692. By Robert Carr on 2013-05-15

Correct terminate handler

691. By Robert Carr on 2013-05-15

Make test pass

690. By Robert Carr on 2013-05-15

Failing test for disabling control sequences

689. By Robert Carr on 2013-05-15

Add a disable control sequences interface to VT

688. By Robert Carr on 2013-05-15

Add a make_raw method to VTFileOperations which disables control sequences

687. By Robert Carr on 2013-05-15

Allow access to the internal egl native display without usage of a surface.

Approved by PS Jenkins bot, Kevin DuBois.

686. By Robert Ancell on 2013-05-15

Release version 0.0.3

685. By Alexandros Frantzis on 2013-05-14

shell: Close all open sessions when destroying the SessionManager

We need to do this manually to break the cyclic dependency between msh::Session
and msh::InputTargetListener, since our implementations of these interfaces
keep strong references to each other. This change fixes LP:#1170643. Fixes: https://bugs.launchpad.net/bugs/1170643.

Approved by Robert Carr, Alan Griffiths, PS Jenkins bot.

684. By Alan Griffiths on 2013-05-14

Remove MIR_GCC_VERSION variable.

This was useful when the Android SDK wasn't up to date. We're now building
on a full Raring/Saucy image, so we don't want to use a non-default compiler.

Approved by Kevin DuBois, Alexandros Frantzis, PS Jenkins bot.

683. By Chris Halse Rogers on 2013-05-14

Fix build with g++ 4.8.

Approved by PS Jenkins bot, Robert Carr.

=== modified file 'examples/demo-shell/demo_shell.cpp'
--- examples/demo-shell/demo_shell.cpp	2013-04-29 17:39:12 +0000
+++ examples/demo-shell/demo_shell.cpp	2013-05-15 18:11:27 +0000
@@ -22,6 +22,7 @@
 #include "fullscreen_placement_strategy.h"
 
 #include "mir/run_mir.h"
+#include "mir/display_server.h"
 #include "mir/report_exception.h"
 #include "mir/default_server_configuration.h"
 #include "mir/shell/session_manager.h"
@@ -33,6 +34,8 @@
 
 #include <iostream>
 
+#include <linux/input.h>
+
 namespace me = mir::examples;
 namespace msh = mir::shell;
 namespace mg = mir::graphics;
@@ -44,6 +47,31 @@
 namespace examples
 {
 
+struct TerminateHandler : mi::EventFilter
+{
+    TerminateHandler() : server(0) {}
+    void set_server(DisplayServer* ds)
+    {
+        server = ds;
+    }
+    
+    bool handles(MirEvent const& ev)
+    {
+        if (ev.type != mir_event_type_key)
+            return false;
+        if (!(ev.key.modifiers & mir_key_modifier_ctrl))
+            return false;
+        if (!(ev.key.modifiers & mir_key_modifier_alt))
+            return false;
+        if (ev.key.scan_code != KEY_BACKSPACE)
+            return false;
+        server->stop();
+        return true;
+    }
+       
+    DisplayServer* server;
+};
+
 struct DemoServerConfiguration : mir::DefaultServerConfiguration
 {
     DemoServerConfiguration(int argc, char const* argv[],
@@ -76,14 +104,17 @@
 int main(int argc, char const* argv[])
 try
 {
+    auto terminate_handler = std::make_shared<me::TerminateHandler>();
     auto app_switcher = std::make_shared<me::ApplicationSwitcher>();
-    me::DemoServerConfiguration config(argc, argv, {app_switcher});
+    auto event_filters = std::initializer_list<std::shared_ptr<mi::EventFilter> const>{terminate_handler, app_switcher};
+    me::DemoServerConfiguration config(argc, argv, event_filters);
     
-    mir::run_mir(config, [&config, &app_switcher](mir::DisplayServer&)
+    mir::run_mir(config, [&](mir::DisplayServer& server)
         {
             // We use this strange two stage initialization to avoid a circular dependency between the EventFilters
             // and the SessionStore
             app_switcher->set_focus_controller(config.the_focus_controller());
+            terminate_handler->set_server(&server);
         });
     return 0;
 }

=== modified file 'include/test/mir_test_doubles/null_virtual_terminal.h'
--- include/test/mir_test_doubles/null_virtual_terminal.h	2013-04-25 09:48:54 +0000
+++ include/test/mir_test_doubles/null_virtual_terminal.h	2013-05-15 18:11:27 +0000
@@ -32,6 +32,7 @@
 {
 public:
     void set_graphics_mode() {}
+    void disable_control_sequences() {}
 
     void register_switch_handlers(MainLoop&,
                                   std::function<bool()> const&,

=== modified file 'src/server/graphics/gbm/gbm_display.cpp'
--- src/server/graphics/gbm/gbm_display.cpp	2013-05-13 09:23:04 +0000
+++ src/server/graphics/gbm/gbm_display.cpp	2013-05-15 18:11:27 +0000
@@ -56,6 +56,7 @@
                        std::make_shared<KMSPageFlipper>(platform->drm.fd)}
 {
     platform->vt->set_graphics_mode();
+    platform->vt->disable_control_sequences();
 
     shared_egl.setup(platform->gbm);
 

=== modified file 'src/server/graphics/gbm/gbm_platform.cpp'
--- src/server/graphics/gbm/gbm_platform.cpp	2013-05-14 16:45:25 +0000
+++ src/server/graphics/gbm/gbm_platform.cpp	2013-05-15 18:11:27 +0000
@@ -29,6 +29,7 @@
 
 #include <fcntl.h>
 #include <sys/ioctl.h>
+#include <termios.h>
 
 namespace mg = mir::graphics;
 namespace mgg = mg::gbm;
@@ -71,6 +72,19 @@
     {
         return ::ioctl(d, request, p_val);
     }
+    
+    int make_raw(int vt_fd)
+    {
+        struct termios terminal_attributes;
+        auto status = tcgetattr(vt_fd, &terminal_attributes);
+        if (status)
+            return status;
+
+        cfmakeraw(&terminal_attributes);
+        terminal_attributes.c_oflag |= OPOST | OCRNL;
+
+        return tcsetattr(vt_fd, TCSANOW, &terminal_attributes);
+    }
 };
 
 }

=== modified file 'src/server/graphics/gbm/linux_virtual_terminal.cpp'
--- src/server/graphics/gbm/linux_virtual_terminal.cpp	2013-04-25 09:48:54 +0000
+++ src/server/graphics/gbm/linux_virtual_terminal.cpp	2013-05-15 18:11:27 +0000
@@ -82,6 +82,17 @@
     }
 }
 
+void mgg::LinuxVirtualTerminal::disable_control_sequences()
+{
+    if (fops->make_raw(vt_fd.fd()))
+    {
+        BOOST_THROW_EXCEPTION(
+            boost::enable_error_info(
+                std::runtime_error("Failed to disable control sequences on VT"))
+                   << boost::errinfo_errno(errno));
+    }
+}
+
 void mgg::LinuxVirtualTerminal::register_switch_handlers(
     MainLoop& main_loop,
     std::function<bool()> const& switch_away,

=== modified file 'src/server/graphics/gbm/linux_virtual_terminal.h'
--- src/server/graphics/gbm/linux_virtual_terminal.h	2013-04-25 09:48:54 +0000
+++ src/server/graphics/gbm/linux_virtual_terminal.h	2013-05-15 18:11:27 +0000
@@ -45,6 +45,7 @@
     virtual int close(int fd) = 0;
     virtual int ioctl(int d, int request, int val) = 0;
     virtual int ioctl(int d, int request, void* p_val) = 0;
+    virtual int make_raw(int vt_fd) = 0;
 
 protected:
     VTFileOperations() = default;
@@ -60,6 +61,7 @@
     ~LinuxVirtualTerminal() noexcept(true);
 
     void set_graphics_mode();
+    void disable_control_sequences();
     void register_switch_handlers(
         MainLoop& main_loop,
         std::function<bool()> const& switch_away,

=== modified file 'src/server/graphics/gbm/virtual_terminal.h'
--- src/server/graphics/gbm/virtual_terminal.h	2013-04-25 09:48:54 +0000
+++ src/server/graphics/gbm/virtual_terminal.h	2013-05-15 18:11:27 +0000
@@ -37,6 +37,7 @@
     virtual ~VirtualTerminal() = default;
 
     virtual void set_graphics_mode() = 0;
+    virtual void disable_control_sequences() = 0;
     virtual void register_switch_handlers(
         MainLoop& main_loop,
         std::function<bool()> const& switch_away,

=== modified file 'tests/unit-tests/graphics/gbm/test_gbm_display.cpp'
--- tests/unit-tests/graphics/gbm/test_gbm_display.cpp	2013-05-10 18:32:25 +0000
+++ tests/unit-tests/graphics/gbm/test_gbm_display.cpp	2013-05-15 18:11:27 +0000
@@ -58,6 +58,7 @@
     ~MockVirtualTerminal() noexcept(true) {}
 
     MOCK_METHOD0(set_graphics_mode, void());
+    MOCK_METHOD0(disable_control_sequences, void());
     MOCK_METHOD3(register_switch_handlers,
                  void(mir::MainLoop&,
                       std::function<bool()> const&,
@@ -627,7 +628,7 @@
     EXPECT_NE(0, callback_count);
 }
 
-TEST_F(GBMDisplayTest, constructor_sets_vt_graphics_mode)
+TEST_F(GBMDisplayTest, constructor_sets_vt_graphics_and_input_mode)
 {
     using namespace testing;
 
@@ -635,6 +636,8 @@
 
     EXPECT_CALL(*mock_vt, set_graphics_mode())
         .Times(1);
+    EXPECT_CALL(*mock_vt, disable_control_sequences())
+        .Times(1);
 
     auto platform = std::make_shared<mgg::GBMPlatform>(null_report, mock_vt);
 

=== modified file 'tests/unit-tests/graphics/gbm/test_linux_virtual_terminal.cpp'
--- tests/unit-tests/graphics/gbm/test_linux_virtual_terminal.cpp	2013-04-18 12:56:58 +0000
+++ tests/unit-tests/graphics/gbm/test_linux_virtual_terminal.cpp	2013-05-15 18:11:27 +0000
@@ -47,6 +47,7 @@
     MOCK_METHOD1(close, int(int));
     MOCK_METHOD3(ioctl, int(int, int, int));
     MOCK_METHOD3(ioctl, int(int, int, void*));
+    MOCK_METHOD1(make_raw, int(int));
 };
 
 class MockMainLoop : public mir::MainLoop
@@ -382,3 +383,53 @@
     /* Fake a VT switch back request */
     sig_handler(SIGUSR1);
 }
+
+TEST_F(LinuxVirtualTerminalTest, disables_control_sequences)
+{
+    using namespace testing;
+
+    int const vt_num{7};
+    int const success = 0;
+
+    InSequence s;
+
+    set_up_expectations_for_current_vt_search(vt_num);
+    set_up_expectations_for_vt_setup(vt_num);
+
+    EXPECT_CALL(mock_fops, make_raw(fake_vt_fd))
+        .WillOnce(Return(success));
+
+    set_up_expectations_for_vt_teardown();
+
+    auto fops = mt::fake_shared<mgg::VTFileOperations>(mock_fops);
+    auto null_report = std::make_shared<mg::NullDisplayReport>();
+
+    mgg::LinuxVirtualTerminal vt(fops, null_report);
+    vt.disable_control_sequences();
+}
+
+TEST_F(LinuxVirtualTerminalTest, failure_to_disable_control_sequences_throws)
+{
+    using namespace testing;
+
+    int const vt_num{7};
+    int const failure = -1;
+
+    InSequence s;
+
+    set_up_expectations_for_current_vt_search(vt_num);
+    set_up_expectations_for_vt_setup(vt_num);
+
+    EXPECT_CALL(mock_fops, make_raw(fake_vt_fd))
+        .WillOnce(Return(failure));
+
+    set_up_expectations_for_vt_teardown();
+
+    auto fops = mt::fake_shared<mgg::VTFileOperations>(mock_fops);
+    auto null_report = std::make_shared<mg::NullDisplayReport>();
+
+    mgg::LinuxVirtualTerminal vt(fops, null_report);
+    EXPECT_THROW({
+            vt.disable_control_sequences();
+    }, std::runtime_error);
+}