Mir

Merge lp://qastaging/~alan-griffiths/mir/we-dont-need-future-here into lp://qastaging/mir

  1. we-dont-need-future-here
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Merged
Approved by: Chris Halse Rogers
Approved revision: no longer in the source branch.
Merged at revision: 3197
Proposed branch: lp://qastaging/~alan-griffiths/mir/we-dont-need-future-here
Merge into: lp://qastaging/mir
Diff against target: 223 lines (+35/-41)
10 files modified
include/server/mir/shell/display_configuration_controller.h (+1/-4)
src/include/server/mir/frontend/display_changer.h (+1/-1)
src/server/frontend/unauthorized_display_changer.cpp (+2/-2)
src/server/frontend/unauthorized_display_changer.h (+1/-1)
src/server/scene/mediating_display_changer.cpp (+2/-8)
src/server/scene/mediating_display_changer.h (+1/-2)
tests/acceptance-tests/test_client_surface_events.cpp (+21/-13)
tests/acceptance-tests/test_display_configuration.cpp (+3/-3)
tests/include/mir/test/doubles/mock_display_changer.h (+2/-2)
tests/include/mir/test/doubles/null_display_changer.h (+1/-5)
To merge this branch: bzr merge lp://qastaging/~alan-griffiths/mir/we-dont-need-future-here
Reviewer Review Type Date Requested Status
Chris Halse Rogers Approve
PS Jenkins bot (community) continuous-integration Approve
Daniel van Vugt Abstain
Kevin DuBois (community) Approve
Alexandros Frantzis (community) Approve
Review via email: mp+279754@code.qastaging.launchpad.net

Commit message

scene, frontend: there is no point in deferring returning from setting the base display config until after it might have been applied

Description of the change

scene, frontend: there is no point in deferring returning from setting the base display config until after it might have been applied

To post a comment you must log in.
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

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

PASSED: Continuous integration, rev:3179
http://jenkins.qa.ubuntu.com/job/mir-ci/5785/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5217
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4123
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5165
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/112/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/114
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/114/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/114
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/114/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5165
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5165/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7695
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25839
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/110/console

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

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

Looks like an ABI break and then you realize it's not.

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/2240/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5230
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4136/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5178
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-autolanding/74
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-autolanding/74/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-autolanding/74
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-autolanding/74/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5178
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5178/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7707
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25866

review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :
Download full text (3.5 KiB)

Failure logged as lp:1523872

6: [ RUN ] TestClientInput.clients_receive_relative_pointer_events
6: [1449566689.901158] mirserver: Starting
6: [1449566689.901295] mirserver: Selected driver: dummy (version 0.19.0)
6: [1449566689.903120] mirserver: Initial display configuration:
6: [1449566689.903138] mirserver: 1.1: VGA 0.0" 0x0mm
6: [1449566689.903146] mirserver: Current mode 1000x800 60.00Hz
6: [1449566689.903149] mirserver: Preferred mode 1000x800 60.00Hz
6: [1449566689.903153] mirserver: Logical position +0+0
6: [1449566689.904287] mirserver: Using software cursor
6: [1449566689.904611] mirserver: Selected input driver: stub-input (version: 0.19.0)
6: [1449566689.904641] mirserver: Mir version 0.19.0
6: /mir/tests/acceptance-tests/test_client_input.cpp:250: Failure
6: Actual function call count doesn't match EXPECT_CALL(first_client, handle_input(mt::PointerEnterEvent()))...
6: Expected: to be called once
6: Actual: never called - unsatisfied and active
6: /mir/tests/acceptance-tests/test_client_input.cpp:251: Failure
6: Actual function call count doesn't match EXPECT_CALL(first_client, handle_input(AllOf(mt::PointerEventWithPosition(1, 1), mt::PointerEventWithDiff(1, 1))))...
6: Expected: to be called once
6: Actual: never called - unsatisfied and active
6: /mir/tests/acceptance-tests/test_client_input.cpp:252: Failure
6: Actual function call count doesn't match EXPECT_CALL(first_client, handle_input(AllOf(mt::PointerEventWithPosition(2, 2), mt::PointerEventWithDiff(1, 1))))...
6: Expected: to be called once
6: Actual: never called - unsatisfied and active
6: /mir/tests/acceptance-tests/test_client_input.cpp:253: Failure
6: Actual function call count doesn't match EXPECT_CALL(first_client, handle_input(AllOf(mt::PointerEventWithPosition(3, 3), mt::PointerEventWithDiff(1, 1))))...
6: Expected: to be called once
6: Actual: never called - unsatisfied and active
6: /mir/tests/acceptance-tests/test_client_input.cpp:254: Failure
6: Actual function call count doesn't match EXPECT_CALL(first_client, handle_input(AllOf(mt::PointerEventWithPosition(2, 2), mt::PointerEventWithDiff(-1, -1))))...
6: Expected: to be called once
6: Actual: never called - unsatisfied and active
6: /mir/tests/acceptance-tests/test_client_input.cpp:255: Failure
6: Actual function call count doesn't match EXPECT_CALL(first_client, handle_input(AllOf(mt::PointerEventWithPosition(1, 1), mt::PointerEventWithDiff(-1, -1))))...
6: Expected: to be called once
6: Actual: never called - unsatisfied and active
6: /mir/tests/acceptance-tests/test_client_input.cpp:257: Failure
6: Actual function call count doesn't match EXPECT_CALL(first_client, handle_input(AllOf(mt::PointerEventWithPosition(0, 0), mt::PointerEventWithDiff(-1, -1))))...
6: Expected: to be called once
6: Actual: never called - unsatisfied and active
6: /mir/tests/acceptance-tests/test_client_input.cpp:258: Failure
6: Actual function call count doesn't match EXPECT_CALL(first_client, handle_input(AllOf(mt::PointerEventWithPosition(0, 0), mt::PointerEventWithDiff(-1, -1))))...
6: Expected: to be called once
6: Actual: never called - unsatisfied and active
6: /mir/tests/acceptance-tests/test_client_input.cpp:259: Failur...

Read more...

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/2242/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5234
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4140
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5182
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-autolanding/76/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-autolanding/76
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-autolanding/76/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5182
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5182/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7711
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25870

review: Needs Fixing (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

lgtm

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Logged the above remaining failure as bug 1524161.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Whoops. RAOF says bug 1524161 is actually cause by this branch.

review: Needs Information
Revision history for this message
Chris Halse Rogers (raof) wrote :

Bug 1524161 appears to be a race introduced by this MP.

Specifically - surface_receives_output_event_on_creation calls display_controller()->set_base_configuration() and then mir_surface_create_sync(), expecting the display configuration to have been set before the surface_create request is processed.

This is going to be hard to hit (I can't seem to hit it locally), because it requires scheduling the test thread, then the server RPC processing thread before the server mainloop processes the ServerAction set up by the set_base_configuration() request.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) :
review: Abstain
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/2246/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5241
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4147/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5189
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-autolanding/80
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-autolanding/80/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-autolanding/80
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-autolanding/80/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5189
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5189/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7718
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25888

review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

And there we go. Different CI job hit the race :).

This could be solved for the test by mir_connection_set_display_config_change_callback and waiting for the callback to be fired, but that's not a solution avaliable to a shell [ie: the thing calling set_base_configuration()].

How would a shell know when the base configuration it requested was applied? I suppose it can guess based on the size of the DisplayBuffers its compositing to...

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

> Bug 1524161 appears to be a race introduced by this MP.
>
> Specifically - surface_receives_output_event_on_creation calls
> display_controller()->set_base_configuration() and then
> mir_surface_create_sync(), expecting the display configuration to have been
> set before the surface_create request is processed.
>
> This is going to be hard to hit (I can't seem to hit it locally), because it
> requires scheduling the test thread, then the server RPC processing thread
> before the server mainloop processes the ServerAction set up by the
> set_base_configuration() request.

Thanks for the analysis - fixed.

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

PASSED: Continuous integration, rev:3182
http://jenkins.qa.ubuntu.com/job/mir-ci/5842/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5303
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4209
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5252
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/153/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/170
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/170/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/170
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/170/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5252
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5252/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7772
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26017
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/151
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/151/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/10/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26025

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

review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

Yeah, that's OK now.

There's still the question of how a shell would know when its configuration has been applied, but we can come to that if and when we need it :).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches