Changeset 55571 in vbox for trunk/src

Timestamp:

Apr 30, 2015 5:04:37 PM (10 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

99943

Message:

DnD: Simplified cancellation logic.

Location:

trunk/src/VBox

Files:

• Additions/WINNT/VBoxTray/VBoxDnD.cpp (modified) (2 diffs)
• Additions/common/VBoxGuestLib/VBoxGuestR3LibDragAndDrop.cpp (modified) (1 diff)
• Additions/x11/VBoxClient/draganddrop.cpp (modified) (7 diffs)
• Main/include/GuestDnDPrivate.h (modified) (3 diffs)
• Main/include/GuestDnDTargetImpl.h (modified) (1 diff)
• Main/src-client/GuestDnDPrivate.cpp (modified) (1 diff)
• Main/src-client/GuestDnDSourceImpl.cpp (modified) (5 diffs)
• Main/src-client/GuestDnDTargetImpl.cpp (modified) (12 diffs)

trunk/src/VBox/Additions/WINNT/VBoxTray/VBoxDnD.cpp

@@ -1424 +1424 @@
     /*
      * Note: Don't clear this->lstAllowedFormats at the moment, as this value is initialized
-     *       on class creation. We might later want to modify the allowed formats in runtime,
+     *       on class creation. We might later want to modify the allowed formats at runtime,
      *       so keep this in mind when implementing this.
      */
@@ -1665 +1665 @@
              * don't support the stuff we do on the guest side, so make sure we
              * don't process invalid messages forever. */
-            if (cMsgSkippedInvalid++ > 3)
+            if (rc == VERR_INVALID_PARAMETER)
+                cMsgSkippedInvalid++;
+            if (cMsgSkippedInvalid > 32)
             {
                 LogRel(("DnD: Too many invalid/skipped messages from host, exiting ...\n"));

trunk/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3LibDragAndDrop.cpp

@@ -552 +552 @@
             }
 
-#if 0
+#if 0 /* Not used yet. */
             if (pCtx->uProtocol >= XXX)
             {

trunk/src/VBox/Additions/x11/VBoxClient/draganddrop.cpp

@@ -1184 +1184 @@
      * Do we have a new window which now is under the cursor?
      */
-    if (   wndCursor != m_wndCur 
+    if (   wndCursor != m_wndCur
         && newVer    != -1)
     {
@@ -1338 +1338 @@
     if (RT_UNLIKELY(xrc == 0))
     {
-        logError(("Error sending SelectionNotify event to window=%#x: %s\n", 
+        logError(("Error sending SelectionNotify event to window=%#x: %s\n",
                   s.xselection.requestor, gX11->xErrorToString(xrc).c_str()));
     }
@@ -1448 +1448 @@
                 }
 
-                /* 
+                /*
                  * Acknowledge the event by sending a status message back to the window.
                  */
@@ -1466 +1466 @@
                 if (RT_UNLIKELY(xRc == 0))
                 {
-                    logError(("Error sending enter XA_XdndStatus to current window=%#x: %s\n", 
+                    logError(("Error sending enter XA_XdndStatus to current window=%#x: %s\n",
                               m_wndCur, gX11->xErrorToString(xRc).c_str()));
                 }
@@ -1492 +1492 @@
                 if (RT_UNLIKELY(xRc == 0))
                 {
-                    logError(("Error sending position XA_XdndStatus to current window=%#x: %s\n", 
+                    logError(("Error sending position XA_XdndStatus to current window=%#x: %s\n",
                               m_wndCur, gX11->xErrorToString(xRc).c_str()));
                 }
@@ -1691 +1691 @@
                     if (RT_UNLIKELY(xrc == 0))
                     {
-                        logError(("Error sending XA_XdndFinished to proxy window=%#x: %s\n", 
+                        logError(("Error sending XA_XdndFinished to proxy window=%#x: %s\n",
                                   m_wndProxy, gX11->xErrorToString(xrc).c_str()));
                     }
@@ -2326 +2326 @@
             if (rc == VERR_INVALID_PARAMETER)
                 cMsgSkippedInvalid++;
-            if (cMsgSkippedInvalid > 3)
+            if (cMsgSkippedInvalid > 32)
             {
                 LogRel(("DnD: Too many invalid/skipped messages from host, exiting ...\n"));

trunk/src/VBox/Main/include/GuestDnDPrivate.h

@@ -110 +110 @@
         lstDirs.clear();
         lstFiles.clear();
+#if 0 /* Currently the scratch buffer will be maintained elswewhere. */
         if (pvScratchBuf)
         {
@@ -116 +117 @@
         }
         cbScratchBuf = 0;
+#else
+        pvScratchBuf = NULL;
+        cbScratchBuf = 0;
+#endif
     }
 
@@ -549 +554 @@
     int getProtocolVersion(uint32_t *puVersion);
 
-    int addMsg(GuestDnDMsg *pMsg)
-    {
-        mDataBase.mListOutgoing.push_back(pMsg);
-        return VINF_SUCCESS;
-    }
-
-    GuestDnDMsg *nextMsg(void)
-    {
-        if (mDataBase.mListOutgoing.empty())
-            return NULL;
-        return mDataBase.mListOutgoing.front();
-    }
-
-    void removeNext(void)
-    {
-        if (!mDataBase.mListOutgoing.empty())
-        {
-            GuestDnDMsg *pMsg = mDataBase.mListOutgoing.front();
-            if (pMsg)
-                delete pMsg;
-            mDataBase.mListOutgoing.pop_front();
-        }
-    }
+    /** @name Functions for handling a simple host HGCM message queue.
+     * @{ */
+    int msgQueueAdd(GuestDnDMsg *pMsg);
+    GuestDnDMsg *msgQueueGetNext(void);
+    void msgQueueRemoveNext(void);
+    void msgQueueClear(void);
+    /** @}  */
+
+    int sendCancel(void);
+    int waitForEvent(RTMSINTERVAL msTimeout, GuestDnDCallbackEvent &Event, GuestDnDResponse *pResp);
 
 protected:

trunk/src/VBox/Main/include/GuestDnDTargetImpl.h

@@ -74 +74 @@
 
     int i_cancelOperation(void);
-    int i_sendData(PSENDDATACTX pCtx);
+    int i_sendData(PSENDDATACTX pCtx, RTMSINTERVAL msTimeout);
     int i_sendDirectory(PSENDDATACTX pCtx, GuestDnDMsg *pMsg, DnDURIObject &aDirectory);
     int i_sendFile(PSENDDATACTX pCtx, GuestDnDMsg *pMsg, DnDURIObject &aFile);
     int i_sendFileData(PSENDDATACTX pCtx, GuestDnDMsg *pMsg, DnDURIObject &aFile);
-    int i_sendURIData(PSENDDATACTX pCtx);
+    int i_sendURIData(PSENDDATACTX pCtx, RTMSINTERVAL msTimeout);
+    int i_sendRawData(PSENDDATACTX pCtx, RTMSINTERVAL msTimeout);
     int i_sendURIDataLoop(PSENDDATACTX pCtx, GuestDnDMsg *pMsg);
 

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

@@ -831 +831 @@
     return rc;
 }
+
+int GuestDnDBase::msgQueueAdd(GuestDnDMsg *pMsg)
+{
+    mDataBase.mListOutgoing.push_back(pMsg);
+    return VINF_SUCCESS;
+}
+
+GuestDnDMsg *GuestDnDBase::msgQueueGetNext(void)
+{
+    if (mDataBase.mListOutgoing.empty())
+        return NULL;
+    return mDataBase.mListOutgoing.front();
+}
+
+void GuestDnDBase::msgQueueRemoveNext(void)
+{
+    if (!mDataBase.mListOutgoing.empty())
+    {
+        GuestDnDMsg *pMsg = mDataBase.mListOutgoing.front();
+        if (pMsg)
+            delete pMsg;
+        mDataBase.mListOutgoing.pop_front();
+    }
+}
+
+void GuestDnDBase::msgQueueClear(void)
+{
+    GuestDnDMsgList::iterator itMsg = mDataBase.mListOutgoing.begin();
+    while (itMsg != mDataBase.mListOutgoing.end())
+    {
+        delete *itMsg;
+    }
+
+    mDataBase.mListOutgoing.clear();
+}
+
+int GuestDnDBase::sendCancel(void)
+{
+    LogFlowFunc(("Sending cancelation request to guest ...\n"));
+
+    GuestDnDMsg MsgCancel;
+    MsgCancel.setType(DragAndDropSvc::HOST_DND_HG_EVT_CANCEL);
+
+    return GuestDnDInst()->hostCall(MsgCancel.getType(), MsgCancel.getCount(), MsgCancel.getParms());
+}
+
+/** @todo GuestDnDResponse *pResp needs to go. */
+int GuestDnDBase::waitForEvent(RTMSINTERVAL msTimeout, GuestDnDCallbackEvent &Event, GuestDnDResponse *pResp)
+{
+    int rc;
+
+    uint64_t tsStart = RTTimeMilliTS();
+    do
+    {
+        /*
+         * Wait until our desired callback triggered the
+         * wait event. As we don't want to block if the guest does not
+         * respond, so do busy waiting here.
+         */
+        rc = Event.Wait(500 /* ms */);
+        if (RT_SUCCESS(rc))
+        {
+            rc = Event.Result();
+            LogFlowFunc(("Callback done, result is %Rrc\n", rc));
+            break;
+        }
+        else if (rc == VERR_TIMEOUT) /* Continue waiting. */
+            rc = VINF_SUCCESS;
+
+        if (   msTimeout != RT_INDEFINITE_WAIT
+            && RTTimeMilliTS() - tsStart > msTimeout)
+        {
+            rc = VERR_TIMEOUT;
+        }
+        else if (pResp->isProgressCanceled())
+        {
+            pResp->setProgress(100 /* Percent */, DragAndDropSvc::DND_PROGRESS_CANCELLED);
+            rc = VERR_CANCELLED;
+        }
+
+    } while (RT_SUCCESS(rc));
+
+    LogFlowFuncLeaveRC(rc);
+    return rc;
+}
 #endif /* VBOX_WITH_DRAG_AND_DROP */
 

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

@@ -781 +781 @@
     if (SUCCEEDED(autoCaller.rc()))
     {
-        rc = pSource->i_receiveData(pTask->getCtx(), RT_INDEFINITE_WAIT);
+        rc = pSource->i_receiveData(pTask->getCtx(), RT_INDEFINITE_WAIT /* msTimeout */);
     }
     else
@@ -838 +838 @@
         rc = pInst->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
         if (RT_SUCCESS(rc))
-        {
-            /*
-             * Wait until our callback i_receiveRawDataCallback triggered the
-             * wait event.
-             */
-            LogFlowFunc(("Waiting for raw data callback (%RU32ms timeout) ...\n", msTimeout));
-            rc = pCtx->mCallback.Wait(msTimeout);
-            LogFlowFunc(("Raw callback done, rc=%Rrc\n", rc));
-        }
+            rc = waitForEvent(msTimeout, pCtx->mCallback, pCtx->mpResp);
 
     } while (0);
@@ -858 +850 @@
 #undef REGISTER_CALLBACK
 #undef UNREGISTER_CALLBACK
+
+    if (rc == VERR_CANCELLED)
+    {
+        int rc2 = sendCancel();
+        AssertRC(rc2);
+    }
 
     LogFlowFuncLeaveRC(rc);
@@ -918 +916 @@
 
         /* Make the initial call to the guest by telling that we initiated the "dropped" event on
-         * the host and therefore now waiting for the actual URI actual data. */
+         * the host and therefore now waiting for the actual URI data. */
         rc = pInst->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
         if (RT_SUCCESS(rc))
-        {
-            /*
-             * Wait until our callback i_receiveURIDataCallback triggered the
-             * wait event.
-             */
-            LogFlowFunc(("Waiting for URI callback (%RU32ms timeout) ...\n", msTimeout));
-            rc = pCtx->mCallback.Wait(msTimeout);
-            LogFlowFunc(("URI callback done, rc=%Rrc\n", rc));
-            if (RT_SUCCESS(rc))
-            {
-                rc = pCtx->mCallback.Result();
-                LogFlowFunc(("Callback result is %Rrc\n", rc));
-            }
-        }
+            rc = waitForEvent(msTimeout, pCtx->mCallback, pCtx->mpResp);
 
     } while (0);
@@ -950 +935 @@
 #undef REGISTER_CALLBACK
 #undef UNREGISTER_CALLBACK
+
+    if (rc == VERR_CANCELLED)
+    {
+        int rc2 = sendCancel();
+        AssertRC(rc2);
+    }
 
     if (RT_FAILURE(rc))

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

@@ -491 +491 @@
     if (SUCCEEDED(autoCaller.rc()))
     {
-        rc = pTarget->i_sendData(pTask->getCtx());
+        rc = pTarget->i_sendData(pTask->getCtx(), RT_INDEFINITE_WAIT /* msTimeout */);
         /* Nothing to do here anymore. */
     }
@@ -601 +601 @@
 }
 
-int GuestDnDTarget::i_sendData(PSENDDATACTX pCtx)
+int GuestDnDTarget::i_sendData(PSENDDATACTX pCtx, RTMSINTERVAL msTimeout)
 {
     AssertPtrReturn(pCtx,  VERR_INVALID_POINTER);
-
-#define DATA_IS_VALID_BREAK(x) \
-    if (!x) \
-    { \
-        LogFlowFunc(("Invalid URI data value for \"" #x "\"\n")); \
-        rc = VERR_INVALID_PARAMETER; \
-        break; \
-    }
 
     GuestDnD *pInst = GuestDnDInst();
@@ -624 +616 @@
     mDataBase.mListOutgoing.clear();
 
-    do
-    {
-        const char *pszFormat = pCtx->mFormat.c_str();
-        DATA_IS_VALID_BREAK(pszFormat);
-        uint32_t cbFormat = pCtx->mFormat.length() + 1;
-
-        /* Do we need to build up a file tree? */
-        bool fHasURIList = DnDMIMEHasFileURLs(pszFormat, cbFormat);
-        if (fHasURIList)
-        {
-            rc = i_sendURIData(pCtx);
-        }
-        else
-        {
-            GuestDnDMsg Msg;
-
-            size_t cbDataTotal = pCtx->mData.vecData.size();
-            DATA_IS_VALID_BREAK(cbDataTotal);
-
-            /* Just copy over the raw data. */
-            Msg.setType(DragAndDropSvc::HOST_DND_HG_SND_DATA);
-            Msg.setNextUInt32(pCtx->mScreenID);
-            Msg.setNextPointer((void *)pCtx->mFormat.c_str(), (uint32_t)pCtx->mFormat.length() + 1);
-            Msg.setNextUInt32((uint32_t)pCtx->mFormat.length() + 1);
-            Msg.setNextPointer((void*)&pCtx->mData.vecData.front(), (uint32_t)cbDataTotal);
-            Msg.setNextUInt32(cbDataTotal);
-
-            LogFlowFunc(("%zu total bytes of raw data to transfer\n", cbDataTotal));
-
-            /* Make the initial call to the guest by sending the actual data. This might
-             * be an URI list which in turn can lead to more data to send afterwards. */
-            rc = pInst->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
-            if (RT_FAILURE(rc))
-                break;
-        }
-
-    } while (0);
+    const char *pszFormat = pCtx->mFormat.c_str();
+    uint32_t cbFormat = pCtx->mFormat.length() + 1;
+
+    /* Do we need to build up a file tree? */
+    bool fHasURIList = DnDMIMEHasFileURLs(pszFormat, cbFormat);
+    if (fHasURIList)
+    {
+        rc = i_sendURIData(pCtx, msTimeout);
+    }
+    else
+    {
+        rc = i_sendRawData(pCtx, msTimeout);
+    }
 
     ASMAtomicWriteBool(&pCtx->mIsActive, false);
@@ -857 +825 @@
             {
                 pMsg = new GuestDnDMsg();
+
                 rc = pThis->i_sendURIDataLoop(pCtx, pMsg);
                 if (RT_SUCCESS(rc))
                 {
-                    rc = pThis->addMsg(pMsg);
+                    rc = pThis->msgQueueAdd(pMsg);
                     if (RT_SUCCESS(rc)) /* Return message type & required parameter count to the guest. */
                     {
@@ -903 +872 @@
             LogFlowFunc(("pCBData->uMsg=%RU32, paParms=%p, cParms=%RU32\n", pCBData->uMsg, pCBData->paParms, pCBData->cParms));
 
-            GuestDnDMsg *pMsg = pThis->nextMsg();
+            GuestDnDMsg *pMsg = pThis->msgQueueGetNext();
             if (pMsg)
             {
@@ -913 +882 @@
                     || pCBData->cParms  != pMsg->getCount())
                 {
+                    /* Start over. */
+                    pThis->msgQueueClear();
+
                     rc = VERR_INVALID_PARAMETER;
                 }
@@ -923 +895 @@
                     {
                         pCBData->cParms = pMsg->getCount();
-                        pThis->removeNext();
+                        pThis->msgQueueRemoveNext();
                     }
                     else
@@ -971 +943 @@
 }
 
-int GuestDnDTarget::i_sendURIData(PSENDDATACTX pCtx)
+int GuestDnDTarget::i_sendURIData(PSENDDATACTX pCtx, RTMSINTERVAL msTimeout)
 {
     AssertPtrReturn(pCtx, VERR_INVALID_POINTER);
@@ -990 +962 @@
     int rc;
 
-#define REGISTER_CALLBACK(x) \
+#define REGISTER_CALLBACK(x)                                        \
     rc = pCtx->mpResp->setCallback(x, i_sendURIDataCallback, pCtx); \
-    if (RT_FAILURE(rc)) \
+    if (RT_FAILURE(rc))                                             \
         return rc;
+
+#define UNREGISTER_CALLBACK(x)                        \
+    {                                                 \
+        int rc2 = pCtx->mpResp->setCallback(x, NULL); \
+        AssertRC(rc2);                                \
+    }
 
     rc = pCtx->mCallback.Reset();
     if (RT_FAILURE(rc))
         return rc;
-
-#define UNREGISTER_CALLBACK(x) \
-    rc = pCtx->mpResp->setCallback(x, NULL); \
-    AssertRC(rc);
 
     /*
@@ -1057 +1031 @@
         size_t    cbData  = strData.length() + 1; /* Include terminating zero. */
 
-        GuestDnDMsg Msg;
-        Msg.setType(DragAndDropSvc::HOST_DND_HG_SND_DATA);
-        Msg.setNextUInt32(pCtx->mScreenID);
-        Msg.setNextPointer((void *)pCtx->mFormat.c_str(), (uint32_t)pCtx->mFormat.length() + 1);
-        Msg.setNextUInt32((uint32_t)pCtx->mFormat.length() + 1);
-        Msg.setNextPointer((void*)strData.c_str(), (uint32_t)cbData);
-        Msg.setNextUInt32((uint32_t)cbData);
-
-        rc = GuestDnDInst()->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
+        GuestDnDMsg MsgSndData;
+        MsgSndData.setType(DragAndDropSvc::HOST_DND_HG_SND_DATA);
+        MsgSndData.setNextUInt32(pCtx->mScreenID);
+        MsgSndData.setNextPointer((void *)pCtx->mFormat.c_str(), (uint32_t)pCtx->mFormat.length() + 1);
+        MsgSndData.setNextUInt32((uint32_t)pCtx->mFormat.length() + 1);
+        MsgSndData.setNextPointer((void*)strData.c_str(), (uint32_t)cbData);
+        MsgSndData.setNextUInt32((uint32_t)cbData);
+
+        rc = GuestDnDInst()->hostCall(MsgSndData.getType(), MsgSndData.getCount(), MsgSndData.getParms());
         if (RT_SUCCESS(rc))
-        {
-            /*
-             * Wait until our callback i_sendURIDataCallback triggered the
-             * wait event.
-             */
-            LogFlowFunc(("Waiting for URI callback ...\n"));
-            rc = pCtx->mCallback.Wait(RT_INDEFINITE_WAIT);
-            LogFlowFunc(("URI callback done, rc=%Rrc\n", rc));
-        }
+            rc = waitForEvent(msTimeout, pCtx->mCallback, pCtx->mpResp);
 
     } while (0);
 
     /*
-     * Unregister callbacksagain.
+     * Unregister callbacks.
      */
     /* Guest callbacks. */
@@ -1094 +1060 @@
 #undef UNREGISTER_CALLBACK
 
+    /*
+     * Now that we've cleaned up tell the guest side to cancel.
+     */
+    if (rc == VERR_CANCELLED)
+    {
+        int rc2 = sendCancel();
+        AssertRC(rc2);
+    }
+
     /* Destroy temporary scratch buffer. */
     if (pvBuf)
@@ -1171 +1146 @@
     }
 
-    if (   pCtx->mpResp
-        && pCtx->mpResp->isProgressCanceled())
-    {
-        LogFlowFunc(("Cancelling ...\n"));
-
-        rc = i_cancelOperation();
-        if (RT_SUCCESS(rc))
-            rc = VERR_CANCELLED;
-    }
-
     LogFlowFuncLeaveRC(rc);
     return rc;
+}
+
+int GuestDnDTarget::i_sendRawData(PSENDDATACTX pCtx, RTMSINTERVAL msTimeout)
+{
+    AssertPtrReturn(pCtx, VERR_INVALID_POINTER);
+    NOREF(msTimeout);
+
+    GuestDnD *pInst = GuestDnDInst();
+    AssertPtr(pInst);
+
+    /* At the moment we only allow up to 64K raw data. */
+    size_t cbDataTotal = pCtx->mData.vecData.size();
+    if (   !cbDataTotal
+        || cbDataTotal > _64K)
+    {
+        return VERR_INVALID_PARAMETER;
+    }
+
+    /* Just copy over the raw data. */
+    GuestDnDMsg Msg;
+    Msg.setType(DragAndDropSvc::HOST_DND_HG_SND_DATA);
+    Msg.setNextUInt32(pCtx->mScreenID);
+    Msg.setNextPointer((void *)pCtx->mFormat.c_str(), (uint32_t)pCtx->mFormat.length() + 1);
+    Msg.setNextUInt32((uint32_t)pCtx->mFormat.length() + 1);
+    Msg.setNextPointer((void*)&pCtx->mData.vecData.front(), (uint32_t)cbDataTotal);
+    Msg.setNextUInt32(cbDataTotal);
+
+    LogFlowFunc(("%zu total bytes of raw data to transfer\n", cbDataTotal));
+
+    return pInst->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
 }