Merge lp:~josharenson/qtmir/allow_duplicate_focusing into lp:qtmir

[Josh Arenson]

Remove check to see if application is already focused before focusing.

Work around lp:1362017 by removing a check to see if an application is already focused before focusing. This forces all applications to go through all steps of obtaining focus, even if the application was already focused.

* Are there any related MPs required for this MP to build/function as expected? Please list.
No

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes, and ran unit tests

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

Fix lp:1362017

This works around an issue where the panel stays in the foreground when it is used to open an app behind it that already has focus by allowing an application that already has focus, to request it again.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:249
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/~josharenson/qtmir/allow_duplicate_focusing/+merge/234556/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/qtmir-ci/70/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-amd64-ci/70
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-armhf-ci/70/console

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

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

CI not happy as you're missing a commit message. Also please attach this MR to the bug.

You can do this via command line with bzr commit --fixes=lp:12345678
or just by opening the bug and using the "Link a related branch" link

Please add a test.
Please add the bits of the MR checklist: https://wiki.ubuntu.com/Process/Merges/Checklists/QtMir

review: Needs Fixing

[Josh Arenson (josharenson)]

While this workaround works, and I don't really have any issues with it, the underlying cause could be related to lp:1353593 as the underlying surface _is_ losing focus (log output below) but the settings application still thinks it has focus.

qtmir.surfaces: MirSurfaceItem::updateMirSurfaceFocus false
qtmir.surfaces: MirSurfaceManager::onSurfaceAttributeChanged - surface= 0xad9066e0 focus=unfocused

If the application was aware of this, the application would _not_ report as already focused, and this WAR wouldn't be needed (bug should resolve itself). Thoughts?

[Gerry Boland (gerboland)]

Don't forget, "surface focus" and "application focus" are different concepts!

[lp:~josharenson/qtmir/allow_duplicate_focusing]

250. By Josh Arenson on 2014-09-16

Add unit test

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:250
http://jenkins.qa.ubuntu.com/job/qtmir-ci/71/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-amd64-ci/71
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-armhf-ci/71
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-armhf-ci/71/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Works! Thanks

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Y
 * Did CI run pass? If not, please explain why.
Y

review: Approve

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2014-09-04 11:11:26 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2014-09-16 00:28:14 +0000
@@ -358,8 +358,6 @@
     }
 
     resumeApplication(application);
-    if (application == m_focusedApplication)
-        return true;
 
     // set state of previously focused app to suspended
     if (m_focusedApplication) {

=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
--- tests/modules/ApplicationManager/application_manager_test.cpp	2014-08-29 10:18:57 +0000
+++ tests/modules/ApplicationManager/application_manager_test.cpp	2014-09-16 00:28:14 +0000
@@ -402,6 +402,35 @@
     EXPECT_EQ(first_session, the_app->session()->session());
 }
 
+TEST_F(ApplicationManagerTests, focused_app_can_rerequest_focus)
+{
+    using namespace ::testing;
+    quint64 a_procId = 5921;
+    const char an_app_id[] = "some_app";
+    QByteArray a_cmd("/usr/bin/app1 --desktop_file_hint=some_app");
+    std::shared_ptr<mir::scene::Surface> aSurface(nullptr);
+
+    ON_CALL(procInfo, command_line(_)).WillByDefault(Return(a_cmd));
+    ON_CALL(appController, appIdHasProcessId(_,_)).WillByDefault(Return(false));
+    
+    bool authed = true;
+
+    std::shared_ptr<mir::scene::Session> a_session = std::make_shared<MockSession>("Oo", a_procId);
+   
+    applicationManager.authorizeSession(a_procId, authed);
+    onSessionStarting(a_session);
+    applicationManager.onSessionCreatedSurface(a_session.get(), aSurface);
+
+    Application * the_app = applicationManager.findApplication(an_app_id);
+    applicationManager.focusApplication(an_app_id);
+
+    EXPECT_EQ(Application::Running, the_app->state());
+    EXPECT_EQ(true, the_app->focused());
+
+    applicationManager.focusApplication(an_app_id);
+    EXPECT_EQ(true, the_app->focused());
+}
+
 TEST_F(ApplicationManagerTests,suspended_suspends_focused_app_and_marks_it_unfocused_in_the_model)
 {
     using namespace ::testing;