Changeset 85537 in vbox for trunk/src/VBox/Main/src-client/GuestDnDTargetImpl.cpp

Timestamp:

Jul 30, 2020 6:55:32 AM (4 years ago)

Author:

vboxsync

Message:

DnD/Main: More fixes and cleanups.

File:

• trunk/src/VBox/Main/src-client/GuestDnDTargetImpl.cpp (modified) (25 diffs)

trunk/src/VBox/Main/src-client/GuestDnDTargetImpl.cpp

@@ -62 +62 @@
     int getRC(void) const { return mRC; }
     bool isOk(void) const { return RT_SUCCESS(mRC); }
-    const ComObjPtr<GuestDnDTarget> &getTarget(void) const { return mTarget; }
 
 protected:
@@ -87 +86 @@
     void handler()
     {
-        GuestDnDTarget::i_sendDataThreadTask(this);
-    }
-
-    virtual ~GuestDnDSendDataTask(void)
-    {
-        if (mpCtx)
-        {
-            delete mpCtx;
-            mpCtx = NULL;
-        }
-    }
-
-
-    GuestDnDSendCtx *getCtx(void) { return mpCtx; }
+        const ComObjPtr<GuestDnDTarget> pThis(mTarget);
+        Assert(!pThis.isNull());
+
+        AutoCaller autoCaller(pThis);
+        if (FAILED(autoCaller.rc()))
+            return;
+
+        int vrc = pThis->i_sendData(mpCtx, RT_INDEFINITE_WAIT /* msTimeout */);
+        if (RT_FAILURE(vrc)) /* In case we missed some error handling within i_sendData(). */
+        {
+            if (vrc != VERR_CANCELLED)
+                LogRel(("DnD: Sending data to guest failed with %Rrc\n", vrc));
+
+            /* Make sure to fire a cancel request to the guest side in case something went wrong. */
+            pThis->sendCancel();
+        }
+    }
+
+    virtual ~GuestDnDSendDataTask(void) { }
 
 protected:
@@ -295 +299 @@
 
     LogRel2(("DnD: Offered formats to guest:\n"));
-    RTCList<RTCString> lstFormats = strFormats.split("\r\n");
+    RTCList<RTCString> lstFormats = strFormats.split(DND_PATH_SEPARATOR);
     for (size_t i = 0; i < lstFormats.size(); i++)
         LogRel2(("DnD: \t%s\n", lstFormats[i].c_str()));
@@ -307 +311 @@
 
     /* Adjust the coordinates in a multi-monitor setup. */
-    int rc = GUESTDNDINST()->adjustScreenCoordinates(aScreenId, &aX, &aY);
+    int rc = GuestDnDInst()->adjustScreenCoordinates(aScreenId, &aX, &aY);
     if (RT_SUCCESS(rc))
     {
@@ -322 +326 @@
         Msg.setNextUInt32(cbFormats);
 
-        rc = GUESTDNDINST()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
+        rc = GuestDnDInst()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
         if (RT_SUCCESS(rc))
         {
-            GuestDnDResponse *pResp = GUESTDNDINST()->response();
+            GuestDnDResponse *pResp = GuestDnDInst()->response();
             if (pResp && RT_SUCCESS(pResp->waitForGuestResponse()))
                 resAction = GuestDnD::toMainAction(pResp->getActionDefault());
@@ -385 +389 @@
     HRESULT hr = S_OK;
 
-    int rc = GUESTDNDINST()->adjustScreenCoordinates(aScreenId, &aX, &aY);
+    int rc = GuestDnDInst()->adjustScreenCoordinates(aScreenId, &aX, &aY);
     if (RT_SUCCESS(rc))
     {
@@ -400 +404 @@
         Msg.setNextUInt32(cbFormats);
 
-        rc = GUESTDNDINST()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
+        rc = GuestDnDInst()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
         if (RT_SUCCESS(rc))
         {
-            GuestDnDResponse *pResp = GUESTDNDINST()->response();
+            GuestDnDResponse *pResp = GuestDnDInst()->response();
             if (pResp && RT_SUCCESS(pResp->waitForGuestResponse()))
                 resAction = GuestDnD::toMainAction(pResp->getActionDefault());
@@ -440 +444 @@
         Msg.setNextUInt32(0); /** @todo ContextID not used yet. */
 
-    int rc = GUESTDNDINST()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
+    int rc = GuestDnDInst()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
     if (RT_SUCCESS(rc))
     {
-        GuestDnDResponse *pResp = GUESTDNDINST()->response();
+        GuestDnDResponse *pResp = GuestDnDInst()->response();
         if (pResp)
             pResp->waitForGuestResponse();
@@ -479 +483 @@
 
     /* Default action is ignoring. */
-    DnDAction_T resAction    = DnDAction_Ignore;
+    DnDAction_T resAct = DnDAction_Ignore;
+    Utf8Str     resFmt;
 
     /* Check & convert the drag & drop actions to HGCM codes. */
@@ -507 +512 @@
 
     /* Adjust the coordinates in a multi-monitor setup. */
-    HRESULT hr = GUESTDNDINST()->adjustScreenCoordinates(aScreenId, &aX, &aY);
+    HRESULT hr = GuestDnDInst()->adjustScreenCoordinates(aScreenId, &aX, &aY);
     if (SUCCEEDED(hr))
     {
@@ -522 +527 @@
         Msg.setNextUInt32(cbFormats);
 
-        int vrc = GUESTDNDINST()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
+        int vrc = GuestDnDInst()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
         if (RT_SUCCESS(vrc))
         {
-            GuestDnDResponse *pResp = GUESTDNDINST()->response();
-            AssertPtr(pResp);
-
-            vrc = pResp->waitForGuestResponse();
-            if (RT_SUCCESS(vrc))
+            GuestDnDResponse *pResp = GuestDnDInst()->response();
+            if (pResp && RT_SUCCESS(pResp->waitForGuestResponse()))
             {
-                resAction = GuestDnD::toMainAction(pResp->getActionDefault());
+                resAct = GuestDnD::toMainAction(pResp->getActionDefault());
 
                 GuestDnDMIMEList lstFormats = pResp->formats();
                 if (lstFormats.size() == 1) /* Exactly one format to use specified? */
                 {
-                    aFormat = lstFormats.at(0);
-                    LogFlowFunc(("resFormat=%s, resAction=%RU32\n", aFormat.c_str(), pResp->getActionDefault()));
+                    resFmt = lstFormats.at(0);
                 }
                 else
@@ -552 +553 @@
         hr = setError(hr, tr("Retrieving drop coordinates failed"));
 
+    LogFlowFunc(("resFmt=%s, resAct=%RU32, vrc=%Rhrc\n", resFmt.c_str(), resAct, hr));
+
     if (SUCCEEDED(hr))
     {
+        aFormat = resFmt;
         if (aResultAction)
-            *aResultAction = resAction;
-    }
-
-    LogFlowFunc(("Returning hr=%Rhrc\n", hr));
+            *aResultAction = resAct;
+    }
+
     return hr;
 #endif /* VBOX_WITH_DRAG_AND_DROP */
-}
-
-/**
- * Thread handler function for sending data to the guest.
- *
- * @param   pTask               Thread task this handler is associated with.
- */
-/* static */
-void GuestDnDTarget::i_sendDataThreadTask(GuestDnDSendDataTask *pTask)
-{
-    LogFlowFunc(("pTask=%p\n", pTask));
-    AssertPtrReturnVoid(pTask);
-
-    const ComObjPtr<GuestDnDTarget> pThis(pTask->getTarget());
-    Assert(!pThis.isNull());
-
-    AutoCaller autoCaller(pThis);
-    if (FAILED(autoCaller.rc()))
-        return;
-
-    int vrc = pThis->i_sendData(pTask->getCtx(), RT_INDEFINITE_WAIT /* msTimeout */);
-    if (RT_FAILURE(vrc)) /* In case we missed some error handling within i_sendData(). */
-    {
-        AssertFailed();
-        LogRel(("DnD: Sending data to guest failed with %Rrc\n", vrc));
-    }
-
-    AutoWriteLock alock(pThis COMMA_LOCKVAL_SRC_POS);
-
-    Assert(pThis->m_DataBase.cTransfersPending);
-    if (pThis->m_DataBase.cTransfersPending)
-        pThis->m_DataBase.cTransfersPending--;
-
-    LogFlowFunc(("pTarget=%p, vrc=%Rrc (ignored)\n", (GuestDnDTarget *)pThis, vrc));
 }
 
@@ -626 +595 @@
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
+    /* Check if this object still is in a pending state and bail out if so. */
+    if (m_fIsPending)
+        return setError(E_FAIL, tr("Current drop operation to guest still in progress"));
+
+    /* Reset our internal state. */
+    i_reset();
+
     /* At the moment we only support one transfer at a time. */
-    if (m_DataBase.cTransfersPending)
-        return setError(E_INVALIDARG, tr("Another drop operation already is in progress"));
-
-    /* Ditto. */
-    GuestDnDResponse *pResp = GUESTDNDINST()->response();
+    if (GuestDnDInst()->getTargetCount())
+        return setError(E_INVALIDARG, tr("Another drag and drop operation to the guest already is in progress"));
+
+    /* Reset progress object. */
+    GuestDnDResponse *pResp = GuestDnDInst()->response();
     AssertPtr(pResp);
-
     HRESULT hr = pResp->resetProgress(m_pGuest);
     if (FAILED(hr))
         return hr;
 
-    GuestDnDSendDataTask *pTask    = NULL;
-    GuestDnDSendCtx      *pSendCtx = NULL;
+    GuestDnDSendDataTask *pTask = NULL;
 
     try
     {
-        /* pSendCtx is passed into SendDataTask where one is deleted in destructor. */
-        pSendCtx = new GuestDnDSendCtx();
-        pSendCtx->pTarget   = this;
-        pSendCtx->pResp     = pResp;
-        pSendCtx->uScreenID = aScreenId;
-
-        pSendCtx->Meta.strFmt = aFormat;
-        pSendCtx->Meta.add(aData);
-
-        /* pTask is responsible for deletion of pSendCtx after creating */
-        pTask = new GuestDnDSendDataTask(this, pSendCtx);
+        mData.mSendCtx.reset();
+
+        mData.mSendCtx.pTarget   = this;
+        mData.mSendCtx.pResp     = pResp;
+        mData.mSendCtx.uScreenID = aScreenId;
+
+        mData.mSendCtx.Meta.strFmt = aFormat;
+        mData.mSendCtx.Meta.add(aData);
+
+        pTask = new GuestDnDSendDataTask(this, &mData.mSendCtx);
         if (!pTask->isOk())
         {
@@ -660 +633 @@
             throw hr = E_FAIL;
         }
-
-        /* Drop write lock before creating thread. */
-        alock.release();
 
         /* This function delete pTask in case of exceptions,
@@ -681 +651 @@
     if (SUCCEEDED(hr))
     {
-        /* Re-acquire write lock. */
-        alock.acquire();
-
-        m_DataBase.cTransfersPending++;
-
+        /* Register ourselves at the DnD manager. */
+        GuestDnDInst()->registerTarget(this);
+
+        /* Return progress to caller. */
         hr = pResp->queryProgressTo(aProgress.asOutParam());
         ComAssertComRC(hr);
     }
     else
-        hr = setError(hr, tr("Starting thread for GuestDnDTarget::i_sendDataThread (%Rhrc)"), hr);
+        hr = setError(hr, tr("Starting thread for GuestDnDTarget failed (%Rhrc)"), hr);
 
     LogFlowFunc(("Returning hr=%Rhrc\n", hr));
@@ -767 +736 @@
 
     return strError;
+}
+
+void GuestDnDTarget::i_reset(void)
+{
+    LogFlowThisFunc(("\n"));
+
+    mData.mSendCtx.reset();
+
+    m_fIsPending = false;
+
+    /* Unregister ourselves from the DnD manager. */
+    GuestDnDInst()->unregisterTarget(this);
 }
 
@@ -780 +761 @@
     AssertPtrReturn(pCtx, VERR_INVALID_POINTER);
 
-    /* Is this context already in sending state? */
-    if (ASMAtomicReadBool(&pCtx->fIsActive))
-        return VERR_WRONG_ORDER;
-    ASMAtomicWriteBool(&pCtx->fIsActive, true);
+    /* Don't allow receiving the actual data until our current transfer is complete. */
+    if (m_fIsPending)
+        return setError(E_FAIL, tr("Current drop operation to guest still in progress"));
 
     /* Clear all remaining outgoing messages. */
@@ -810 +790 @@
     }
 
-    ASMAtomicWriteBool(&pCtx->fIsActive, false);
+    if (RT_FAILURE(rc))
+        LogRel(("DnD: Sending data to guest failed with %Rrc\n", rc));
+
+    /* Reset state. */
+    i_reset();
 
     LogFlowFuncLeaveRC(rc);
@@ -839 +823 @@
 
 #ifdef DEBUG
-    RTCList<RTCString> lstFilesURI = RTCString((char *)pvData, cbData).split("\r\n");
+    RTCList<RTCString> lstFilesURI = RTCString((char *)pvData, cbData).split(DND_PATH_SEPARATOR);
     LogFlowFunc(("lstFilesURI=%zu\n", lstFilesURI.size()));
     for (size_t i = 0; i < lstFilesURI.size(); i++)
@@ -871 +855 @@
         }
 
-        rc = GUESTDNDINST()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
+        rc = GuestDnDInst()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
         if (RT_FAILURE(rc))
             break;
@@ -907 +891 @@
 
     LogRel2(("DnD: Sending meta data header to guest (%RU64 bytes total data, %RU32 bytes meta data, %RU64 objects)\n",
-             pCtx->getTotal(), pCtx->Meta.cbData, pCtx->Transfer.cObjToProcess));
+             pCtx->getTotalAnnounced(), pCtx->Meta.cbData, pCtx->Transfer.cObjToProcess));
 
     Msg.setNextUInt32(0);                                                /** @todo uContext; not used yet. */
     Msg.setNextUInt32(0);                                                /** @todo uFlags; not used yet. */
     Msg.setNextUInt32(pCtx->uScreenID);                                  /* uScreen */
-    Msg.setNextUInt64(pCtx->getTotal());                                 /* cbTotal */
+    Msg.setNextUInt64(pCtx->getTotalAnnounced());                        /* cbTotal */
     Msg.setNextUInt32((uint32_t)pCtx->Meta.cbData);                      /* cbMeta*/
     Msg.setNextPointer(unconst(pCtx->Meta.strFmt.c_str()), (uint32_t)pCtx->Meta.strFmt.length() + 1); /* pvMetaFmt */
@@ -922 +906 @@
     Msg.setNextUInt32(0);                                                /** @todo cbChecksum; not used yet. */
 
-    int rc = GUESTDNDINST()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
+    int rc = GuestDnDInst()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
 
     LogFlowFuncLeaveRC(rc);
@@ -1398 +1382 @@
          * Update internal state to reflect everything we need to work with it.
          */
-        pCtx->cbExtra               = DnDTransferListObjTotalBytes(&pCtx->Transfer.List);
+        pCtx->cbExtra                = DnDTransferListObjTotalBytes(&pCtx->Transfer.List);
         /* cbExtra can be 0, if all files are of 0 bytes size. */
         pCtx->Transfer.cObjToProcess = DnDTransferListObjCount(&pCtx->Transfer.List);
@@ -1431 +1415 @@
             pCtx->Meta.cbData      = cbData;
             pCtx->Meta.cbAllocated = cbData;
+            pCtx->Meta.cbAnnounced = cbData;
         }
 
@@ -1530 +1515 @@
     AssertRCReturn(rc, rc);
 
-    if (pCtx->isComplete())
-    {
-        Assert(pCtx->Transfer.isComplete());
+    PDNDTRANSFEROBJECT pObj = DnDTransferListObjGetFirst(pList);
+    if (!pObj) /* Transfer complete? */
         return VINF_EOF;
-    }
-
-    PDNDTRANSFEROBJECT pObj = DnDTransferListObjGetFirst(pList);
-    AssertPtrReturn(pObj, VERR_WRONG_ORDER);
 
     switch (DnDTransferObjectGetType(pObj))