Changeset 55963 in vbox

Timestamp:

May 20, 2015 11:18:14 AM (9 years ago)

Author:

vboxsync

Message:

DnD: Fixed hang when reporting host errors such as inaccessible (locked) files.

Location:

trunk

Files:

• include/VBox/err.h (modified) (1 diff)
• src/VBox/Main/include/GuestDnDPrivate.h (modified) (3 diffs)
• src/VBox/Main/include/GuestDnDSourceImpl.h (modified) (1 diff)
• src/VBox/Main/include/GuestDnDTargetImpl.h (modified) (1 diff)
• src/VBox/Main/src-client/GuestDnDPrivate.cpp (modified) (7 diffs)
• src/VBox/Main/src-client/GuestDnDSourceImpl.cpp (modified) (7 diffs)
• src/VBox/Main/src-client/GuestDnDTargetImpl.cpp (modified) (9 diffs)

trunk/include/VBox/err.h

@@ -2646 +2646 @@
 /** @} */
 
+/** @name VBox Drag and Drop Status Codes
+ * @{
+ */
+/** Guest side reported an error. */
+#define VERR_GSTDND_GUEST_ERROR                     (-6500)
+/** @} */
+
 
 /* SED-END */

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

@@ -50 +50 @@
     int Reset(void);
 
-    int Notify(int rc);
+    int Notify(int rc = VINF_SUCCESS);
 
     int Result(void) const { return mRc; }
@@ -367 +367 @@
     bool isProgressCanceled(void) const;
     int setCallback(uint32_t uMsg, PFNGUESTDNDCALLBACK pfnCallback, void *pvUser = NULL);
-    int setProgress(unsigned uPercentage, uint32_t uState, int rcOp = VINF_SUCCESS);
+    int setProgress(unsigned uPercentage, uint32_t uState, int rcOp = VINF_SUCCESS, const Utf8Str &strMsg = "");
     HRESULT resetProgress(const ComObjPtr<Guest>& pParent);
     HRESULT queryProgressTo(IProgress **ppProgress);
@@ -377 +377 @@
     int onDispatch(uint32_t u32Function, void *pvParms, uint32_t cbParms);
     /** @}  */
-
-public:
-
-    Utf8Str errorToString(const ComObjPtr<Guest>& pGuest, int guestRc);
 
 protected:

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

@@ -74 +74 @@
 protected:
 
+    static Utf8Str i_guestErrorToString(int guestRc);
+    static Utf8Str i_hostErrorToString(int hostRc);
+
+    /** @name Thread callbacks.
+     * @{ */
     static DECLCALLBACK(int) i_receiveDataThread(RTTHREAD Thread, void *pvUser);
+    /** @}  */
 
     /** @name Callbacks for dispatch handler.

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

@@ -68 +68 @@
 protected:
 
+    static Utf8Str i_guestErrorToString(int guestRc);
+    static Utf8Str i_hostErrorToString(int hostRc);
+
+    /** @name Thread callbacks.
+     * @{ */
     static DECLCALLBACK(int) i_sendDataThread(RTTHREAD Thread, void *pvUser);
+    /** @}  */
+
+    /** @name Callbacks for dispatch handler.
+     * @{ */
     static DECLCALLBACK(int) i_sendURIDataCallback(uint32_t uMsg, void *pvParms, size_t cbParms, void *pvUser);
+    /** @}  */
 
 protected:

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

@@ -177 +177 @@
 }
 
-int GuestDnDCallbackEvent::Notify(int rc)
+int GuestDnDCallbackEvent::Notify(int rc /* = VINF_SUCCESS */)
 {
     mRc = rc;
@@ -209 +209 @@
 }
 
-/* static */
-Utf8Str GuestDnDResponse::errorToString(const ComObjPtr<Guest>& pGuest, int guestRc)
-{
-    Utf8Str strError;
-
-    switch (guestRc)
-    {
-        case VERR_ACCESS_DENIED:
-            strError += Utf8StrFmt(pGuest->tr("For one or more guest files or directories selected for transferring to the host your guest "
-                                              "user does not have the appropriate access rights for. Please make sure that all selected "
-                                              "elements can be accessed and that your guest user has the appropriate rights."));
-            break;
-
-        case VERR_NOT_FOUND:
-            /* Should not happen due to file locking on the guest, but anyway ... */
-            strError += Utf8StrFmt(pGuest->tr("One or more guest files or directories selected for transferring to the host were not"
-                                              "found on the guest anymore. This can be the case if the guest files were moved and/or"
-                                              "altered while the drag and drop operation was in progress."));
-            break;
-
-        case VERR_SHARING_VIOLATION:
-            strError += Utf8StrFmt(pGuest->tr("One or more guest files or directories selected for transferring to the host were locked. "
-                                              "Please make sure that all selected elements can be accessed and that your guest user has "
-                                              "the appropriate rights."));
-            break;
-
-        default:
-            strError += Utf8StrFmt("Drag and drop guest error (%Rrc)", guestRc);
-            break;
-    }
-
-    return strError;
-}
-
 int GuestDnDResponse::notifyAboutGuestResponse(void) const
 {
@@ -312 +278 @@
 
 int GuestDnDResponse::setProgress(unsigned uPercentage,
-                                  uint32_t uStatus, int rcOp /* = VINF_SUCCESS */)
-{
-    LogFlowFunc(("uStatus=%RU32, uPercentage=%RU32, rcOp=%Rrc\n",
-                 uStatus, uPercentage, rcOp));
+                                  uint32_t uStatus,
+                                  int rcOp /* = VINF_SUCCESS */, const Utf8Str &strMsg /* = "" */)
+{
+    LogFlowFunc(("uStatus=%RU32, uPercentage=%RU32, rcOp=%Rrc, strMsg=%s\n",
+                 uStatus, uPercentage, rcOp, strMsg.c_str()));
 
     int rc = VINF_SUCCESS;
@@ -338 +305 @@
                     hr = m_progress->i_notifyComplete(VBOX_E_IPRT_ERROR,
                                                       COM_IIDOF(IGuest),
-                                                      m_parent->getComponentName(),
-                                                      GuestDnDResponse::errorToString(m_parent, rcOp).c_str());
+                                                      m_parent->getComponentName(), strMsg.c_str());
                     reset();
                     break;
@@ -898 +864 @@
          * 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.
+         * respond, do busy waiting here.
          */
         rc = Event.Wait(500 /* ms */);
@@ -908 +874 @@
         }
         else if (rc == VERR_TIMEOUT) /* Continue waiting. */
-            rc = VINF_SUCCESS;
+            continue;
 
         if (   msTimeout != RT_INDEFINITE_WAIT
@@ -914 +880 @@
         {
             rc = VERR_TIMEOUT;
-        }
-        else if (pResp->isProgressCanceled())
-        {
-            pResp->setProgress(100 /* Percent */, DragAndDropSvc::DND_PROGRESS_CANCELLED);
+            LogFlowFunc(("Guest did not respond within time\n"));
+        }
+        else if (pResp->isProgressCanceled()) /** @todo GuestDnDResponse *pResp needs to go. */
+        {
+            LogFlowFunc(("Canceled by user\n"));
             rc = VERR_CANCELLED;
         }

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

@@ -325 +325 @@
         RecvDataTask *pTask = new RecvDataTask(this, &mData.mRecvCtx);
         AssertReturn(pTask->isOk(), pTask->getRC());
-        
+
         LogFlowFunc(("Starting thread ...\n"));
 
@@ -415 +415 @@
 
 #ifdef VBOX_WITH_DRAG_AND_DROP_GH
+/* static */
+Utf8Str GuestDnDSource::i_guestErrorToString(int guestRc)
+{
+    Utf8Str strError;
+
+    switch (guestRc)
+    {
+        case VERR_ACCESS_DENIED:
+            strError += Utf8StrFmt(tr("For one or more guest files or directories selected for transferring to the host your guest "
+                                      "user does not have the appropriate access rights for. Please make sure that all selected "
+                                      "elements can be accessed and that your guest user has the appropriate rights."));
+            break;
+
+        case VERR_NOT_FOUND:
+            /* Should not happen due to file locking on the guest, but anyway ... */
+            strError += Utf8StrFmt(tr("One or more guest files or directories selected for transferring to the host were not"
+                                      "found on the guest anymore. This can be the case if the guest files were moved and/or"
+                                      "altered while the drag and drop operation was in progress."));
+            break;
+
+        case VERR_SHARING_VIOLATION:
+            strError += Utf8StrFmt(tr("One or more guest files or directories selected for transferring to the host were locked. "
+                                      "Please make sure that all selected elements can be accessed and that your guest user has "
+                                      "the appropriate rights."));
+            break;
+
+        default:
+            strError += Utf8StrFmt(tr("Drag and drop error from guest (%Rrc)"), guestRc);
+            break;
+    }
+
+    return strError;
+}
+
+
+/* static */
+Utf8Str GuestDnDSource::i_hostErrorToString(int hostRc)
+{
+    Utf8Str strError;
+
+    switch (hostRc)
+    {
+        case VERR_ACCESS_DENIED:
+            strError += Utf8StrFmt(tr("For one or more host files or directories selected for transferring to the guest your host "
+                                      "user does not have the appropriate access rights for. Please make sure that all selected "
+                                      "elements can be accessed and that your host user has the appropriate rights."));
+            break;
+
+        case VERR_NOT_FOUND:
+            /* Should not happen due to file locking on the host, but anyway ... */
+            strError += Utf8StrFmt(tr("One or more host files or directories selected for transferring to the host were not"
+                                      "found on the host anymore. This can be the case if the host files were moved and/or"
+                                      "altered while the drag and drop operation was in progress."));
+            break;
+
+        case VERR_SHARING_VIOLATION:
+            strError += Utf8StrFmt(tr("One or more host files or directories selected for transferring to the guest were locked. "
+                                      "Please make sure that all selected elements can be accessed and that your host user has "
+                                      "the appropriate rights."));
+            break;
+
+        default:
+            strError += Utf8StrFmt(tr("Drag and drop error from host (%Rrc)"), hostRc);
+            break;
+    }
+
+    return strError;
+}
+
 int GuestDnDSource::i_onReceiveData(PRECVDATACTX pCtx, const void *pvData, uint32_t cbData, uint64_t cbTotalSize)
 {
@@ -839 +908 @@
         rc = pInst->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
         if (RT_SUCCESS(rc))
+        {
             rc = waitForEvent(msTimeout, pCtx->mCallback, pCtx->mpResp);
+            if (RT_FAILURE(rc))
+            {
+                if (rc == VERR_CANCELLED)
+                    rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_CANCELLED, VINF_SUCCESS);
+                else if (rc != VERR_GSTDND_GUEST_ERROR) /* Guest-side error are already handled in the callback. */
+                    rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_ERROR, rc,
+                                                   GuestDnDSource::i_hostErrorToString(rc));
+            }
+            else
+                rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_COMPLETE, VINF_SUCCESS);
+        }
 
     } while (0);
@@ -919 +1000 @@
         rc = pInst->hostCall(Msg.getType(), Msg.getCount(), Msg.getParms());
         if (RT_SUCCESS(rc))
+        {
             rc = waitForEvent(msTimeout, pCtx->mCallback, pCtx->mpResp);
+            if (RT_FAILURE(rc))
+            {
+                if (rc == VERR_CANCELLED)
+                    rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_CANCELLED, VINF_SUCCESS);
+                else if (rc != VERR_GSTDND_GUEST_ERROR) /* Guest-side error are already handled in the callback. */
+                    rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_ERROR, rc,
+                                                   GuestDnDSource::i_hostErrorToString(rc));
+            }
+            else
+                rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_COMPLETE, VINF_SUCCESS);
+        }
 
     } while (0);
@@ -971 +1064 @@
     int rc = VINF_SUCCESS;
 
+    int rcCallback = VINF_SUCCESS; /* rc for the callback. */
+    bool fNotify = false;
+
     switch (uMsg)
     {
@@ -992 +1088 @@
 
             pCtx->mpResp->reset();
-            rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_ERROR, pCBData->rc);
+
+            if (RT_SUCCESS(pCBData->rc))
+                pCBData->rc = VERR_GENERAL_FAILURE; /* Make sure some error is set. */
+
+            rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_ERROR, pCBData->rc,
+                                           GuestDnDSource::i_guestErrorToString(pCBData->rc));
             if (RT_SUCCESS(rc))
-                rc = pCBData->rc;
+                rcCallback = VERR_GSTDND_GUEST_ERROR;
             break;
         }
@@ -1118 +1219 @@
     }
 
+    if (RT_FAILURE(rc))
+    {
+        switch (rc)
+        {
+            case VERR_NO_DATA:
+                LogRel2(("DnD: Transfer to host complete\n"));
+                break;
+
+            case VERR_CANCELLED:
+                LogRel2(("DnD: Transfer to host canceled\n"));
+                break;
+
+            default:
+                LogRel(("DnD: Error %Rrc occurred, aborting transfer to host\n", rc));
+                break;
+        }
+
+        /* Unregister this callback. */
+        AssertPtr(pCtx->mpResp);
+        int rc2 = pCtx->mpResp->setCallback(uMsg, NULL /* PFNGUESTDNDCALLBACK */);
+        AssertRC(rc2);
+    }
+
     /* All URI data processed? */
     if (pCtx->mData.cbProcessed >= pCtx->mData.cbToProcess)

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

@@ -605 +605 @@
 }
 
+/* static */
+Utf8Str GuestDnDTarget::i_guestErrorToString(int guestRc)
+{
+    Utf8Str strError;
+
+    switch (guestRc)
+    {
+        case VERR_ACCESS_DENIED:
+            strError += Utf8StrFmt(tr("For one or more guest files or directories selected for transferring to the host your guest "
+                                      "user does not have the appropriate access rights for. Please make sure that all selected "
+                                      "elements can be accessed and that your guest user has the appropriate rights."));
+            break;
+
+        case VERR_NOT_FOUND:
+            /* Should not happen due to file locking on the guest, but anyway ... */
+            strError += Utf8StrFmt(tr("One or more guest files or directories selected for transferring to the host were not"
+                                      "found on the guest anymore. This can be the case if the guest files were moved and/or"
+                                      "altered while the drag and drop operation was in progress."));
+            break;
+
+        case VERR_SHARING_VIOLATION:
+            strError += Utf8StrFmt(tr("One or more guest files or directories selected for transferring to the host were locked. "
+                                      "Please make sure that all selected elements can be accessed and that your guest user has "
+                                      "the appropriate rights."));
+            break;
+
+        default:
+            strError += Utf8StrFmt(tr("Drag and drop error from guest (%Rrc)"), guestRc);
+            break;
+    }
+
+    return strError;
+}
+
+/* static */
+Utf8Str GuestDnDTarget::i_hostErrorToString(int hostRc)
+{
+    Utf8Str strError;
+
+    switch (hostRc)
+    {
+        case VERR_ACCESS_DENIED:
+            strError += Utf8StrFmt(tr("For one or more host files or directories selected for transferring to the guest your host "
+                                      "user does not have the appropriate access rights for. Please make sure that all selected "
+                                      "elements can be accessed and that your host user has the appropriate rights."));
+            break;
+
+        case VERR_NOT_FOUND:
+            /* Should not happen due to file locking on the host, but anyway ... */
+            strError += Utf8StrFmt(tr("One or more host files or directories selected for transferring to the host were not"
+                                      "found on the host anymore. This can be the case if the host files were moved and/or"
+                                      "altered while the drag and drop operation was in progress."));
+            break;
+
+        case VERR_SHARING_VIOLATION:
+            strError += Utf8StrFmt(tr("One or more host files or directories selected for transferring to the guest were locked. "
+                                      "Please make sure that all selected elements can be accessed and that your host user has "
+                                      "the appropriate rights."));
+            break;
+
+        default:
+            strError += Utf8StrFmt(tr("Drag and drop error from host (%Rrc)"), hostRc);
+            break;
+    }
+
+    return strError;
+}
+
 int GuestDnDTarget::i_sendData(PSENDDATACTX pCtx, RTMSINTERVAL msTimeout)
 {
-    AssertPtrReturn(pCtx,  VERR_INVALID_POINTER);
+    AssertPtrReturn(pCtx, VERR_INVALID_POINTER);
 
     GuestDnD *pInst = GuestDnDInst();
@@ -682 +750 @@
                           RTFILE_O_OPEN | RTFILE_O_READ | RTFILE_O_DENY_WRITE, 0 /* fFlags */);
         if (RT_FAILURE(rc))
-            LogRel2(("DnD: Error opening host file \"%s\", rc=%Rrc\n", strPathSrc.c_str(), rc));
+            LogRel(("DnD: Error opening host file \"%s\", rc=%Rrc\n", strPathSrc.c_str(), rc));
     }
 
@@ -814 +882 @@
     LogFlowFunc(("pThis=%p, uMsg=%RU32\n", pThis, uMsg));
 
-    int rc = VINF_SUCCESS;
+    int rc = VINF_SUCCESS; /* Will be reported back to guest. */
+
+    int rcCallback = VINF_SUCCESS; /* rc for the callback. */
+    bool fNotify = false;
 
     switch (uMsg)
@@ -858 +929 @@
 
             pCtx->mpResp->reset();
-            rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_ERROR, pCBData->rc);
+
+            if (RT_SUCCESS(pCBData->rc))
+                pCBData->rc = VERR_GENERAL_FAILURE; /* Make sure some error is set. */
+
+            rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_ERROR, pCBData->rc,
+                                           GuestDnDTarget::i_guestErrorToString(pCBData->rc));
             if (RT_SUCCESS(rc))
-                rc = pCBData->rc;
+                rcCallback = VERR_GSTDND_GUEST_ERROR;
             break;
         }
@@ -915 +991 @@
     }
 
+    if (   RT_FAILURE(rc)
+        || RT_FAILURE(rcCallback))
+    {
+        fNotify = true;
+        if (RT_SUCCESS(rcCallback))
+            rcCallback = rc;
+    }
+
     if (RT_FAILURE(rc))
     {
@@ -920 +1004 @@
         {
             case VERR_NO_DATA:
-                LogRel2(("DnD: Transfer complete\n"));
+                LogRel2(("DnD: Transfer to guest complete\n"));
                 break;
 
             case VERR_CANCELLED:
-                LogRel2(("DnD: Transfer canceled\n"));
+                LogRel2(("DnD: Transfer to guest canceled\n"));
                 break;
 
             default:
-                LogRel(("DnD: Error %Rrc occurred, aborting transfer\n", rc));
+                LogRel(("DnD: Error %Rrc occurred, aborting transfer to guest\n", rc));
                 break;
         }
@@ -936 +1020 @@
         int rc2 = pCtx->mpResp->setCallback(uMsg, NULL /* PFNGUESTDNDCALLBACK */);
         AssertRC(rc2);
-
-        /* Notify waiters. */
-        rc2 = pCtx->mCallback.Notify(rc);
+    }
+
+    LogFlowFunc(("fNotify=%RTbool, rcCallback=%Rrc, rc=%Rrc\n", fNotify, rcCallback, rc));
+
+    if (fNotify)
+    {
+        int rc2 = pCtx->mCallback.Notify(rcCallback);
         AssertRC(rc2);
     }
@@ -1044 +1132 @@
         rc = GuestDnDInst()->hostCall(MsgSndData.getType(), MsgSndData.getCount(), MsgSndData.getParms());
         if (RT_SUCCESS(rc))
+        {
             rc = waitForEvent(msTimeout, pCtx->mCallback, pCtx->mpResp);
+            if (RT_FAILURE(rc))
+            {
+                if (rc == VERR_CANCELLED)
+                    rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_CANCELLED, VINF_SUCCESS);
+                else if (rc != VERR_GSTDND_GUEST_ERROR) /* Guest-side error are already handled in the callback. */
+                    rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_ERROR, rc,
+                                                   GuestDnDTarget::i_hostErrorToString(rc));
+            }
+            else
+                rc = pCtx->mpResp->setProgress(100, DragAndDropSvc::DND_PROGRESS_COMPLETE, VINF_SUCCESS);
+        }
 
     } while (0);
@@ -1065 +1165 @@
     /*
      * Now that we've cleaned up tell the guest side to cancel.
+     * This does not imply we're waiting for the guest to react, as the
+     * host side never must depend on anything from the guest.
      */
     if (rc == VERR_CANCELLED)