Merge lp:~aacid/qtubuntu/make_sure_mimedata_is_good into lp:qtubuntu

[Albert Astals Cid]

Make sure mMimeData doesn't point to already deleted memory

QMirClientClipboard::mimeData(QClipboard::Mode mode) calls updateMimeData() and then returns mMimeData

Given that updateMimeData() can end up deleting the data in mMimeData but not always resetting the pointer we may end up returning a pointer that points to garbage

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

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

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

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:372
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/192/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4271
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4299
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4133
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4133/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4133
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4133/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4133
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4133/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4133
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4133/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4133
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4133/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4133
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4133/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/192/rebuild

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

Why don't we put the "delete mMimeData" inside the "if (focusWindow)" instead? That way we keep the old data until we're able to update it.

[Albert Astals Cid (aacid)]

> Why don't we put the "delete mMimeData" inside the "if (focusWindow)" instead?
> That way we keep the old data until we're able to update it.

I've the feeling that it makes more sense to return nothing if there's nothing on the clipboard than old stuff, but I have not much field expertize.

[Daniel d'Andrada (dandrader)]

We don't know if there's nothing on the clipboard as weren't able to fetch it in the first place due to content-hub security restrictions (which require requestor to be focused at the moment of request)

[Albert Astals Cid (aacid)]

> We don't know if there's nothing on the clipboard as weren't able to fetch it in the first place
> due to content-hub security restrictions (which require requestor to be focused at the moment of
> request)

Shouldn't that be checked in contentHub and not in qtubuntu?

[Daniel d'Andrada (dandrader)]

> > We don't know if there's nothing on the clipboard as weren't able to fetch
> it in the first place
> > due to content-hub security restrictions (which require requestor to be
> focused at the moment of
> > request)
>
> Shouldn't that be checked in contentHub and not in qtubuntu?

contentHub does check it. qtubuntu just doesn't bother making the request when it knows it will fail.

[lp:~aacid/qtubuntu/make_sure_mimedata_is_good]

373. By Albert Astals Cid on 2017-03-02

Daniel prefers this

[Albert Astals Cid (aacid)]

> Why don't we put the "delete mMimeData" inside the "if (focusWindow)" instead?
> That way we keep the old data until we're able to update it.

Ok

[Daniel d'Andrada (dandrader)]

thanks

review: Approve

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:373
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/194/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4280
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4308
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4142
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4142/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4142
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4142/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4142
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4142/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4142
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4142/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4142
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4142/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4142
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4142/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/194/rebuild

review: Approve (continuous-integration)

=== modified file 'src/ubuntumirclient/qmirclientclipboard.cpp'
--- src/ubuntumirclient/qmirclientclipboard.cpp	2017-02-07 15:37:20 +0000
+++ src/ubuntumirclient/qmirclientclipboard.cpp	2017-03-02 10:49:39 +0000
@@ -141,10 +141,9 @@
         return;
     }
 
-    delete mMimeData;
-
     QWindow *focusWindow = QGuiApplication::focusWindow();
     if (focusWindow) {
+        delete mMimeData;
         QString surfaceId = static_cast<QMirClientWindow*>(focusWindow->handle())->persistentSurfaceId();
         mMimeData = mContentHub->latestPaste(surfaceId);
         mClipboardState = SyncedClipboard;