Merge lp:~robertcarr/mir/1233944-addendum into lp:~mir-team/mir/trunk

[kevin gunn]

Fix for LP#1233944

Fixes the Mir-side of bug https://bugs.launchpad.net/mir/+bug/1233944

Event files are first created with root:root permissions and only later
udev rules are applied to it, changing its permissions to root:android-input
and therefore making it readable by unity8-mir

in short: Retry opening a file when its permissions change as it might be
readable now.

Reproposing from my addendum branch as we want to land this quickly and dandrader is EOD. Thanks dandrader :D

==
Fixes bug 1233944.

But further work might be required (likely outside mir) due to the following error message when android-input loads the autopilot-finger device file:

"""
[InputReader] Touch device 'autopilot-finger' did not report support for X or Y axis! The device will be inoperable.
"""

[kevin gunn (kgunn72)]

this change is already approved on lp:~mir-team/mir/development-branch
proposing to trunk to fast track outside of dev branch merger

[Robert Ancell (robert-ancell)]

As discussed on IRC - this is already merged into the development-branch, so it's OK to merge straight into trunk to save time doing the double merge.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1100
http://jenkins.qa.ubuntu.com/job/mir-ci/1578/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2220
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2105
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/822/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/79
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/79/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

> As discussed on IRC - this is already merged into the development-branch,
> so it's OK to merge straight into trunk to save time doing the double merge.

Although this will probably work fine in most cases, it complicates the history/relationship between lp:mir and the development-branch, making more difficult to figure out what's in each branch that's not in the other. I suggest that we don't make a habit of this, and do the double merge (fix=>devel=>lp:mir) in the future. Doing so keeps things conceptually clean. If we need to fast track we can perform one of the merges manually (i.e. avoiding a second CI run).

[Daniel van Vugt (vanvugt)]

Agreed. And I gasped for a moment before realizing the same was already in development-branch.

Life should get easier when we abolish development-branch.

=== modified file '3rd_party/android-input/android/frameworks/base/services/input/EventHub.cpp'
--- 3rd_party/android-input/android/frameworks/base/services/input/EventHub.cpp	2013-05-28 17:52:18 +0000
+++ 3rd_party/android-input/android/frameworks/base/services/input/EventHub.cpp	2013-10-03 04:45:31 +0000
@@ -223,7 +223,7 @@
     LOG_ALWAYS_FATAL_IF(mEpollFd < 0, "Could not create epoll instance.  errno=%d", errno);
 
     mINotifyFd = inotify_init();
-    int result = inotify_add_watch(mINotifyFd, DEVICE_PATH, IN_DELETE | IN_CREATE);
+    int result = inotify_add_watch(mINotifyFd, DEVICE_PATH, IN_DELETE | IN_CREATE | IN_ATTRIB);
     LOG_ALWAYS_FATAL_IF(result < 0, "Could not register INotify for %s.  errno=%d",
             DEVICE_PATH, errno);
 
@@ -955,6 +955,11 @@
     char buffer[80];
 
     ALOGV("Opening device: %s", devicePath);
+    if (hasDeviceByPathLocked(String8(devicePath)))
+    {
+        ALOGV("Not opening device (%s), as it is already opened", devicePath);
+        return -1;
+    }
 
     int fd = open(devicePath, O_RDWR | O_CLOEXEC);
     if(fd < 0) {
@@ -1416,6 +1421,14 @@
             strcpy(filename, event->name);
             if(event->mask & IN_CREATE) {
                 openDeviceLocked(devname);
+            } else if (event->mask & IN_ATTRIB) {
+                Device* device = getDeviceByPathLocked(devname);
+                if (!device) {
+                    ALOGI("Retry opening device file %s", devname);
+                    // file permissions might have changed, making the device readable now
+                    // let's retry opening it
+                    openDeviceLocked(devname);
+                }
             } else {
                 ALOGI("Removing device '%s' due to inotify event\n", devname);
                 closeDeviceByPathLocked(devname);
@@ -1523,4 +1536,13 @@
     }
 }
 
+bool EventHub::hasDeviceByPathLocked(String8 const& path)
+{
+    for (size_t i = 0; i < mDevices.size(); i++) {
+        auto const& device = mDevices.valueAt(i);
+        if (device->path == path) return true;
+    }
+    return false;
+}
+
 }; // namespace android

=== modified file '3rd_party/android-input/android/frameworks/base/services/input/EventHub.h'
--- 3rd_party/android-input/android/frameworks/base/services/input/EventHub.h	2013-05-28 17:47:03 +0000
+++ 3rd_party/android-input/android/frameworks/base/services/input/EventHub.h	2013-10-03 04:45:31 +0000
@@ -395,6 +395,7 @@
 
     int32_t mNextDeviceId;
 
+    bool hasDeviceByPathLocked(String8 const& path);
     KeyedVector<int32_t, Device*> mDevices;
 
     Device *mOpeningDevices;