Merge lp:~alan-griffiths/mir/nested-server-applies-display-config-policy into lp:mir

[Alan Griffiths]

nested: Apply the display config policy during startup

nested: Apply the display config policy during startup

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3050
http://jenkins.qa.ubuntu.com/job/mir-ci/5388/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4620/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3527/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4562/console
    None: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/520/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1542
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1542/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/520
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/520/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4563/console
    None: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/521/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3051
http://jenkins.qa.ubuntu.com/job/mir-ci/5389/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4622
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3529
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4564
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/522/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1543
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1543/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/522
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/522/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4565
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4565/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7189
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24518
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/523/console

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

Before my final approval, though, I would like for Andreas to review because I remember him mentioning that applying the config at startup had created problems for the greeter in the past (something to do with power modes but I don't remember the details).

review: Needs Information

[Andreas Pokorny (andreas-pokorny)]

Looks like a good opportunity to use the new shiny std::equa std::equal which takes two ranges in c++14

[Andreas Pokorny (andreas-pokorny)]

@alf: The sequence of what happens with the separated greeter process was:
 - usc receives the trigger to turn the screen off
 - changes the config to turn the screen off
 - usc/lightm? starts a greeter process to be on top when the display is turned on again
 - (greeter is a nested server with indicators as clients..)
 - greeter submits a display configuration (like in the MP here)
 - usc focuses the greeter session
 - on focus change the sessions display configuration with powermode == on is applied
 - graphics platform turns screen on again..

a cleaner solution would have been to make usc mediate the display change. Currently we dont have a split out nested greeter so the problem will not occur immediately...

So apart from the std::equal comment I am happy with this..

review: Approve

[Andreas Pokorny (andreas-pokorny)]

oh even better..

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[Kevin DuBois (kdub)]

lgtm too

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3052
http://jenkins.qa.ubuntu.com/job/mir-ci/5395/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4631
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3538
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4573/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/531/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1549
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1549/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/531
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/531/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4574
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4574/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7198/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24550
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/532/console

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

CI glitch?

+ sudo add-apt-repository --yes ppa:ci-train-ppa-service/stable-phone-overlay
Cannot add PPA: 'ppa:~ci-train-ppa-service/ubuntu/stable-phone-overlay'.
The team named '~ci-train-ppa-service' has no PPA named 'ubuntu/stable-phone-overlay'
Please choose from the following available PPAs:
...

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'include/platform/mir/graphics/display_configuration.h'
--- include/platform/mir/graphics/display_configuration.h	2015-10-05 05:22:49 +0000
+++ include/platform/mir/graphics/display_configuration.h	2015-10-26 12:54:41 +0000
@@ -181,6 +181,9 @@
     DisplayConfiguration& operator=(DisplayConfiguration const& c) = delete;
 };
 
+bool operator==(DisplayConfiguration const& lhs, DisplayConfiguration const& rhs);
+bool operator!=(DisplayConfiguration const& lhs, DisplayConfiguration const& rhs);
+
 std::ostream& operator<<(std::ostream& out, DisplayConfiguration const& val);
 
 }

=== modified file 'src/platform/graphics/display_configuration.cpp'
--- src/platform/graphics/display_configuration.cpp	2015-10-02 05:14:37 +0000
+++ src/platform/graphics/display_configuration.cpp	2015-10-26 12:54:41 +0000
@@ -145,6 +145,7 @@
 
     out << "\tscale: " << val.scale << std::endl;
     out << "\tform factor: " << as_string(val.form_factor) << std::endl;
+    out << "\torientation: " << val.orientation << '\n';
     out << "}" << std::endl;
 
     return out;
@@ -219,6 +220,28 @@
     return !(val1 == val2);
 }
 
+bool mg::operator==(DisplayConfiguration const& lhs, DisplayConfiguration const& rhs)
+{
+    std::vector<DisplayConfigurationCard> lhs_cards;
+    std::vector<DisplayConfigurationOutput> lhs_outputs;
+
+    lhs.for_each_card([&lhs_cards](DisplayConfigurationCard const& card) { lhs_cards.emplace_back(card); });
+    lhs.for_each_output([&lhs_outputs](DisplayConfigurationOutput const& output) { lhs_outputs.emplace_back(output); });
+
+    std::vector<DisplayConfigurationCard> rhs_cards;
+    std::vector<DisplayConfigurationOutput> rhs_outputs;
+
+    rhs.for_each_card([&rhs_cards](DisplayConfigurationCard const& card) { rhs_cards.emplace_back(card); });
+    rhs.for_each_output([&rhs_outputs](DisplayConfigurationOutput const& output) { rhs_outputs.emplace_back(output); });
+
+    return lhs_cards == rhs_cards && lhs_outputs == rhs_outputs;
+}
+
+bool mg::operator!=(DisplayConfiguration const& lhs, DisplayConfiguration const& rhs)
+{
+    return !(lhs == rhs);
+}
+
 namespace
 {
 mir::geometry::Rectangle extents_of(

=== modified file 'src/server/graphics/nested/display.cpp'
--- src/server/graphics/nested/display.cpp	2015-10-06 09:28:48 +0000
+++ src/server/graphics/nested/display.cpp	2015-10-26 12:54:41 +0000
@@ -163,6 +163,10 @@
 {
     std::shared_ptr<DisplayConfiguration> conf(configuration());
     initial_conf_policy->apply_to(*conf);
+
+    if (*configuration() != *conf)
+        apply_to_connection(*conf);
+
     create_surfaces(*conf);
 }
 
@@ -195,8 +199,11 @@
 
 void mgn::Display::configure(mg::DisplayConfiguration const& configuration)
 {
-    create_surfaces(configuration);
-    apply_to_connection(configuration);
+    if (*this->configuration() != configuration)
+    {
+        apply_to_connection(configuration);
+        create_surfaces(configuration);
+    }
 }
 
 void mgn::Display::create_surfaces(mg::DisplayConfiguration const& configuration)

=== modified file 'tests/acceptance-tests/test_nested_mir.cpp'
--- tests/acceptance-tests/test_nested_mir.cpp	2015-10-21 10:35:51 +0000
+++ tests/acceptance-tests/test_nested_mir.cpp	2015-10-26 12:54:41 +0000
@@ -22,6 +22,7 @@
 #include "mir/input/cursor_images.h"
 #include "mir/graphics/display.h"
 #include "mir/graphics/display_configuration.h"
+#include "mir/graphics/display_configuration_policy.h"
 #include "mir/graphics/display_configuration_report.h"
 #include "mir/input/cursor_listener.h"
 #include "mir/cached_ptr.h"
@@ -183,10 +184,35 @@
     bool hidden{false};
 };
 
+struct MockDisplayConfigurationPolicy : mg::DisplayConfigurationPolicy
+{
+    MOCK_METHOD1(apply_to, void (mg::DisplayConfiguration&));
+};
+
 class NestedMirRunner : public mtf::HeadlessNestedServerRunner
 {
 public:
     NestedMirRunner(std::string const& connection_string)
+        : NestedMirRunner(connection_string, true)
+    {
+        start_server();
+    }
+
+    virtual ~NestedMirRunner()
+    {
+        stop_server();
+    }
+
+    std::shared_ptr<MockHostLifecycleEventListener> the_mock_host_lifecycle_event_listener()
+    {
+        return mock_host_lifecycle_event_listener([]
+            { return std::make_shared<NiceMock<MockHostLifecycleEventListener>>(); });
+    }
+
+    std::shared_ptr<CursorWrapper> cursor_wrapper;
+
+protected:
+    NestedMirRunner(std::string const& connection_string, bool)
         : mtf::HeadlessNestedServerRunner(connection_string)
     {
         server.override_the_host_lifecycle_event_listener([this]
@@ -197,24 +223,21 @@
 
         server.override_the_cursor_images([] { return std::make_shared<CursorImages>(); });
 
-        start_server();
-    }
-
-    ~NestedMirRunner()
-    {
-        stop_server();
-    }
-
-    std::shared_ptr<MockHostLifecycleEventListener> the_mock_host_lifecycle_event_listener()
-    {
-        return mock_host_lifecycle_event_listener([]
-            { return std::make_shared<NiceMock<MockHostLifecycleEventListener>>(); });
-    }
-
-    std::shared_ptr<CursorWrapper> cursor_wrapper;
+        server.wrap_display_configuration_policy([this]
+            (std::shared_ptr<mg::DisplayConfigurationPolicy> const&)
+            { return mock_display_configuration_policy([this]
+                { return build_mock_display_configuration_policy(); }); });
+    }
 
 private:
     mir::CachedPtr<MockHostLifecycleEventListener> mock_host_lifecycle_event_listener;
+    
+    virtual std::shared_ptr<MockDisplayConfigurationPolicy> build_mock_display_configuration_policy()
+    {
+        return std::make_shared<NiceMock<MockDisplayConfigurationPolicy>>();
+    }
+
+    mir::CachedPtr<MockDisplayConfigurationPolicy> mock_display_configuration_policy;
 };
 
 struct NestedServer : mtf::HeadlessInProcessServer
@@ -376,6 +399,7 @@
     // No post on surface creation
     EXPECT_CALL(*mock_session_mediator_report, session_submit_buffer_called(_)).Times(0);
     NestedMirRunner nested_mir{new_connection()};
+
     auto const connection = mir_connect_sync(nested_mir.new_connection().c_str(), __PRETTY_FUNCTION__);
     auto const surface = mtf::make_any_surface(connection);
 
@@ -618,3 +642,48 @@
     // surface as the host cursor then reverts to default.
     Mock::VerifyAndClearExpectations(mock_cursor.get());
 }
+
+TEST_F(NestedServer, applies_display_config_on_startup)
+{
+    mt::WaitCondition condition;
+
+    auto const expected_config = server.the_display()->configuration();
+    expected_config->for_each_output([](mg::UserDisplayConfigurationOutput& output)
+        { output.orientation = mir_orientation_inverted;});
+
+    EXPECT_CALL(*the_mock_display_configuration_report(), new_configuration(mt::DisplayConfigMatches(std::ref(*expected_config))))
+        .WillRepeatedly(InvokeWithoutArgs([&] { condition.wake_up_everyone(); }));
+
+    struct MyNestedMirRunner : NestedMirRunner
+    {
+        MyNestedMirRunner(std::string const& connection_string) :
+            NestedMirRunner(connection_string, true)
+        {
+            start_server();
+        }
+
+        std::shared_ptr<MockDisplayConfigurationPolicy> build_mock_display_configuration_policy() override
+        {
+            auto result = std::make_unique<MockDisplayConfigurationPolicy>();
+            EXPECT_CALL(*result, apply_to(_)).Times(AnyNumber())
+                .WillOnce(Invoke([](mg::DisplayConfiguration& config)
+                     {
+                        config.for_each_output([](mg::UserDisplayConfigurationOutput& output)
+                            { output.orientation = mir_orientation_inverted; });
+                     }));
+
+            return std::shared_ptr<MockDisplayConfigurationPolicy>(std::move(result));
+        }
+    } nested_mir{new_connection()};
+
+    auto const connection = mir_connect_sync(nested_mir.new_connection().c_str(), __PRETTY_FUNCTION__);
+    auto const surface = make_and_paint_surface(connection);
+
+    condition.wait_for_at_most_seconds(1);
+    Mock::VerifyAndClearExpectations(the_mock_display_configuration_report().get());
+
+    EXPECT_TRUE(condition.woken());
+
+    mir_surface_release_sync(surface);
+    mir_connection_release(connection);
+}