Merge lp:~afrantzis/mir/fix-1344024 into lp:mir

[Alexandros Frantzis]

server: Allow clients to create surfaces even when there are no active outputs (LP: #1344024)

This is fixed by removing obsolete placement rules from our default placement
policy. These rules had the side-effect of prohibiting surface creation when
there were no active outputs.

server: Allow clients to create surfaces even when there are no active outputs

This is fixed by removing obsolete placement rules from our default placement
policy. These rules had the side-effect of prohibiting surface creation when
there were no active outputs.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1799
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2248/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1118
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1103
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/770
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/770/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/771
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/771/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/38
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/38/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2225
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10531

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1800
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2252/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1116
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1122
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1107
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/774
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/774/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/775
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/775/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/42
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/42/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2230
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10548

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Maybe the default could be better, but this will do for now.

review: Approve

=== modified file 'src/server/shell/CMakeLists.txt'
--- src/server/shell/CMakeLists.txt	2014-05-26 22:00:52 +0000
+++ src/server/shell/CMakeLists.txt	2014-07-28 13:50:21 +0000
@@ -2,7 +2,7 @@
   SHELL_SOURCES
 
   default_focus_mechanism.cpp
-  consuming_placement_strategy.cpp
+  default_placement_strategy.cpp
   graphics_display_layout.cpp
   default_configuration.cpp
   session_coordinator_wrapper.cpp

=== modified file 'src/server/shell/default_configuration.cpp'
--- src/server/shell/default_configuration.cpp	2014-07-21 03:35:31 +0000
+++ src/server/shell/default_configuration.cpp	2014-07-28 13:50:21 +0000
@@ -20,7 +20,7 @@
 #include "null_host_lifecycle_event_listener.h"
 
 
-#include "consuming_placement_strategy.h"
+#include "default_placement_strategy.h"
 #include "default_focus_mechanism.h"
 #include "graphics_display_layout.h"
 
@@ -34,7 +34,7 @@
     return shell_placement_strategy(
         [this]
         {
-            return std::make_shared<msh::ConsumingPlacementStrategy>(
+            return std::make_shared<msh::DefaultPlacementStrategy>(
                 the_shell_display_layout());
         });
 }

=== renamed file 'src/server/shell/consuming_placement_strategy.cpp' => 'src/server/shell/default_placement_strategy.cpp'
--- src/server/shell/consuming_placement_strategy.cpp	2014-04-15 05:31:19 +0000
+++ src/server/shell/default_placement_strategy.cpp	2014-07-28 13:50:21 +0000
@@ -16,7 +16,7 @@
  * Authored by: Robert Carr <[e-mail]>
  */
 
-#include "consuming_placement_strategy.h"
+#include "default_placement_strategy.h"
 #include "mir/scene/surface_creation_parameters.h"
 #include "mir/shell/display_layout.h"
 #include "mir/geometry/rectangle.h"
@@ -29,13 +29,13 @@
 namespace msh = mir::shell;
 namespace geom = mir::geometry;
 
-msh::ConsumingPlacementStrategy::ConsumingPlacementStrategy(
+msh::DefaultPlacementStrategy::DefaultPlacementStrategy(
     std::shared_ptr<msh::DisplayLayout> const& display_layout)
     : display_layout(display_layout)
 {
 }
 
-ms::SurfaceCreationParameters msh::ConsumingPlacementStrategy::place(
+ms::SurfaceCreationParameters msh::DefaultPlacementStrategy::place(
     ms::Session const& /* session */,
     ms::SurfaceCreationParameters const& request_parameters)
 {
@@ -49,15 +49,6 @@
     {
         display_layout->place_in_output(request_parameters.output_id, rect);
     }
-    else if (request_parameters.size.width > geom::Width{0} &&
-             request_parameters.size.height > geom::Height{0})
-    {
-        display_layout->clip_to_output(rect);
-    }
-    else
-    {
-        display_layout->size_to_output(rect);
-    }
 
     placed_parameters.top_left = rect.top_left;
     placed_parameters.size = rect.size;

=== renamed file 'src/server/shell/consuming_placement_strategy.h' => 'src/server/shell/default_placement_strategy.h'
--- src/server/shell/consuming_placement_strategy.h	2014-04-15 05:31:19 +0000
+++ src/server/shell/default_placement_strategy.h	2014-07-28 13:50:21 +0000
@@ -16,8 +16,8 @@
  * Authored by: Robert Carr <[e-mail]>
  */
 
-#ifndef MIR_SHELL_CONSUMING_PLACEMENT_STRATEGY_H_
-#define MIR_SHELL_CONSUMING_PLACEMENT_STRATEGY_H_
+#ifndef MIR_SHELL_DEFAULT_PLACEMENT_STRATEGY_H_
+#define MIR_SHELL_DEFAULT_PLACEMENT_STRATEGY_H_
 
 #include "mir/scene/placement_strategy.h"
 
@@ -29,18 +29,18 @@
 {
 class DisplayLayout;
 
-class ConsumingPlacementStrategy : public scene::PlacementStrategy
+class DefaultPlacementStrategy : public scene::PlacementStrategy
 {
 public:
-    explicit ConsumingPlacementStrategy(
+    explicit DefaultPlacementStrategy(
             std::shared_ptr<DisplayLayout> const& display_layout);
-    virtual ~ConsumingPlacementStrategy() {}
+    virtual ~DefaultPlacementStrategy() {}
 
     virtual scene::SurfaceCreationParameters place(scene::Session const& session, scene::SurfaceCreationParameters const& request_parameters);
 
 protected:
-    ConsumingPlacementStrategy(ConsumingPlacementStrategy const&) = delete;
-    ConsumingPlacementStrategy& operator=(ConsumingPlacementStrategy const&) = delete;
+    DefaultPlacementStrategy(DefaultPlacementStrategy const&) = delete;
+    DefaultPlacementStrategy& operator=(DefaultPlacementStrategy const&) = delete;
 
 private:
     std::shared_ptr<DisplayLayout> const display_layout;
@@ -49,4 +49,4 @@
 }
 } // namespace mir
 
-#endif // MIR_SHELL_CONSUMING_PLACEMENT_STRATEGY_H_
+#endif // MIR_SHELL_DEFAULT_PLACEMENT_STRATEGY_H_

=== modified file 'tests/acceptance-tests/CMakeLists.txt'
--- tests/acceptance-tests/CMakeLists.txt	2014-07-23 17:00:22 +0000
+++ tests/acceptance-tests/CMakeLists.txt	2014-07-28 13:50:21 +0000
@@ -41,6 +41,7 @@
   test_client_with_custom_display_config_deadlock.cpp
   test_macros.cpp
   test_stale_frames.cpp
+  test_server_without_active_outputs.cpp
   ${GENERATED_PROTOBUF_SRCS}
   ${GENERATED_PROTOBUF_HDRS}
 )

=== modified file 'tests/acceptance-tests/test_focus_selection.cpp'
--- tests/acceptance-tests/test_focus_selection.cpp	2014-06-03 11:04:15 +0000
+++ tests/acceptance-tests/test_focus_selection.cpp	2014-07-28 13:50:21 +0000
@@ -19,7 +19,6 @@
 #include "clients.h"
 
 #include "src/server/scene/session_container.h"
-#include "src/server/shell/consuming_placement_strategy.h"
 #include "src/server/scene/session_manager.h"
 #include "mir/graphics/display.h"
 #include "mir/shell/input_targeter.h"

=== added file 'tests/acceptance-tests/test_server_without_active_outputs.cpp'
--- tests/acceptance-tests/test_server_without_active_outputs.cpp	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/test_server_without_active_outputs.cpp	2014-07-28 13:50:21 +0000
@@ -0,0 +1,64 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Alexandros Frantzis <[e-mail]>
+ */
+
+#include "mir_toolkit/mir_client_library.h"
+
+#include "mir_test_framework/stubbed_server_configuration.h"
+#include "mir_test_framework/basic_client_server_fixture.h"
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+namespace mtf = mir_test_framework;
+namespace geom = mir::geometry;
+
+namespace
+{
+
+struct NoOutputsServerConfig : mtf::StubbedServerConfiguration
+{
+    NoOutputsServerConfig()
+        : mtf::StubbedServerConfiguration(std::vector<geom::Rectangle>{})
+    {
+    }
+};
+
+using ServerWithoutActiveOutputs = mtf::BasicClientServerFixture<NoOutputsServerConfig>;
+
+}
+
+TEST_F(ServerWithoutActiveOutputs, creates_valid_client_surface)
+{
+    using namespace testing;
+
+    MirSurfaceParameters const request_params =
+    {
+        __PRETTY_FUNCTION__,
+        640, 480,
+        mir_pixel_format_abgr_8888,
+        mir_buffer_usage_hardware,
+        mir_display_output_id_invalid
+    };
+
+    auto const surface = mir_connection_create_surface_sync(connection, &request_params);
+
+    EXPECT_THAT(mir_surface_is_valid(surface), Eq(mir_true))
+        << mir_surface_get_error_message(surface);
+
+    mir_surface_release_sync(surface);
+}

=== modified file 'tests/integration-tests/session_management.cpp'
--- tests/integration-tests/session_management.cpp	2014-06-03 11:04:15 +0000
+++ tests/integration-tests/session_management.cpp	2014-07-28 13:50:21 +0000
@@ -98,6 +98,7 @@
     std::shared_ptr<mf::EventSink> const event_sink = std::make_shared<mtd::NullEventSink>();
     std::shared_ptr<mf::Shell> const session_manager = builder.the_frontend_shell();
     std::shared_ptr<TestSurfaceStack> const& test_surface_stack = builder.test_surface_stack;
+    ms::SurfaceCreationParameters const params = ms::SurfaceCreationParameters().of_size(100,100);
 
     void SetUp()
     {
@@ -117,8 +118,6 @@
 
 TEST_F(SessionManagement, creating_a_surface_adds_it_to_scene)
 {
-    ms::SurfaceCreationParameters params;
-
     auto const session = session_manager->open_session(0, __PRETTY_FUNCTION__, event_sink);
 
     EXPECT_CALL(*test_surface_stack, add_surface(_,_,_)).Times(1);
@@ -129,8 +128,6 @@
 {
     EXPECT_CALL(*test_surface_stack, add_surface(_,_,_)).Times(AnyNumber());
 
-    ms::SurfaceCreationParameters params;
-
     auto const session1 = session_manager->open_session(0, __PRETTY_FUNCTION__, event_sink);
     auto const surface1 = session1->create_surface(params);
 

=== modified file 'tests/mir_test_framework/stubbed_server_configuration.cpp'
--- tests/mir_test_framework/stubbed_server_configuration.cpp	2014-06-23 22:25:17 +0000
+++ tests/mir_test_framework/stubbed_server_configuration.cpp	2014-07-28 13:50:21 +0000
@@ -116,6 +116,13 @@
  public:
     std::shared_ptr<mg::Buffer> alloc_buffer(mg::BufferProperties const& properties) override
     {
+        if (properties.size.width == geom::Width{0} ||
+            properties.size.height == geom::Height{0})
+        {
+            BOOST_THROW_EXCEPTION(
+                std::runtime_error("Request for allocation of buffer with invalid size"));
+        }
+
         return std::make_shared<StubFDBuffer>(properties);
     }
 };

=== modified file 'tests/unit-tests/shell/CMakeLists.txt'
--- tests/unit-tests/shell/CMakeLists.txt	2014-04-15 05:31:19 +0000
+++ tests/unit-tests/shell/CMakeLists.txt	2014-07-28 13:50:21 +0000
@@ -1,5 +1,5 @@
 list(APPEND UNIT_TEST_SOURCES
-  ${CMAKE_CURRENT_SOURCE_DIR}/test_consuming_placement_strategy.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_default_placement_strategy.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_graphics_display_layout.cpp
 )
 

=== renamed file 'tests/unit-tests/shell/test_consuming_placement_strategy.cpp' => 'tests/unit-tests/shell/test_default_placement_strategy.cpp'
--- tests/unit-tests/shell/test_consuming_placement_strategy.cpp	2014-04-15 05:31:19 +0000
+++ tests/unit-tests/shell/test_default_placement_strategy.cpp	2014-07-28 13:50:21 +0000
@@ -19,7 +19,7 @@
 #include "mir_test_doubles/mock_display_layout.h"
 #include "mir_test_doubles/stub_scene_session.h"
 
-#include "src/server/shell/consuming_placement_strategy.h"
+#include "src/server/shell/default_placement_strategy.h"
 #include "mir/scene/surface_creation_parameters.h"
 
 #include "mir/geometry/rectangle.h"
@@ -35,7 +35,7 @@
 namespace
 {
 
-struct ConsumingPlacementStrategySetup : public testing::Test
+struct DefaultPlacementStrategySetup : public testing::Test
 {
     void SetUp()
     {
@@ -48,37 +48,7 @@
 };
 }
 
-
-TEST_F(ConsumingPlacementStrategySetup, parameters_with_no_geometry_are_made_fullscreen)
-{
-    using namespace ::testing;
-
-    ms::SurfaceCreationParameters input_params;
-    geom::Rectangle rect{input_params.top_left, input_params.size};
-
-    EXPECT_CALL(*display_layout, size_to_output(rect)).Times(1);
-
-    msh::ConsumingPlacementStrategy placement_strategy(display_layout);
-
-    placement_strategy.place(session, input_params);
-}
-
-TEST_F(ConsumingPlacementStrategySetup, parameters_with_geometry_are_clipped)
-{
-    using namespace ::testing;
-
-    ms::SurfaceCreationParameters input_params;
-    input_params.size = geom::Size{100, 200};
-    geom::Rectangle rect{input_params.top_left, input_params.size};
-
-    EXPECT_CALL(*display_layout, clip_to_output(rect)).Times(1);
-
-    msh::ConsumingPlacementStrategy placement_strategy(display_layout);
-
-    placement_strategy.place(session, input_params);
-}
-
-TEST_F(ConsumingPlacementStrategySetup, parameters_with_output_id_are_placed_in_output)
+TEST_F(DefaultPlacementStrategySetup, parameters_with_output_id_are_placed_in_output)
 {
     using namespace ::testing;
 
@@ -91,7 +61,7 @@
                 place_in_output(input_params.output_id, rect))
         .Times(1);
 
-    msh::ConsumingPlacementStrategy placement_strategy(display_layout);
+    msh::DefaultPlacementStrategy placement_strategy(display_layout);
 
     placement_strategy.place(session, input_params);
 }