Changeset 29934 in vbox for trunk/src/VBox/Main/ProgressProxyImpl.cpp

Timestamp:

Jun 1, 2010 6:39:21 AM (15 years ago)

Author:

vboxsync

Message:

reverted r62200 as it breaks powerup

File:

• trunk/src/VBox/Main/ProgressProxyImpl.cpp (modified) (19 diffs)

trunk/src/VBox/Main/ProgressProxyImpl.cpp

@@ -46 +46 @@
 HRESULT ProgressProxy::FinalConstruct()
 {
-    mfMultiOperation = false;
-    muOtherProgressStartWeight = 0;
-    muOtherProgressWeight = 0;
-    muOtherProgressStartOperation = 0;
+    mcOtherProgressObjects = 1;
+    miCurOtherProgressObject = 0;
 
     HRESULT rc = Progress::FinalConstruct();
@@ -68 +66 @@
                             BOOL fCancelable)
 {
-    mfMultiOperation = false;
-    muOtherProgressStartWeight = 1;
-    muOtherProgressWeight = 1;
-    muOtherProgressStartOperation = 1;
+    miCurOtherProgressObject = 0;
+    mcOtherProgressObjects = 0;
 
     return Progress::init(
@@ -88 +84 @@
 
 /**
- * Initialize for proxying one other progress object.
- *
- * This is tailored explicitly for the openRemoteSession code, so we start out
- * with one operation where we don't have any remote object (powerUp).  Then a
- * remote object is added and stays with us till the end.
- *
- * The user must do normal completion notification or risk leave the threads
- * waiting forever!
+ * Initialize for proxying one or more other objects, assuming we start out and
+ * end without proxying anyone.
+ *
+ * The user must call clearOtherProgressObject when there are no more progress
+ * objects to be proxied or we'll leave threads waiting forever.
  */
 HRESULT ProgressProxy::init(
@@ -104 +97 @@
                             CBSTR bstrDescription,
                             BOOL fCancelable,
+                            ULONG cOtherProgressObjects,
                             ULONG uTotalOperationsWeight,
                             CBSTR bstrFirstOperationDescription,
                             ULONG uFirstOperationWeight,
-                            ULONG cOtherProgressObjectOperations)
-{
-    mfMultiOperation = false;
-    muOtherProgressStartWeight    = uFirstOperationWeight;
-    muOtherProgressWeight         = uTotalOperationsWeight - uFirstOperationWeight;
-    muOtherProgressStartOperation = 1;
+                            OUT_GUID pId)
+{
+    miCurOtherProgressObject = 0;
+    mcOtherProgressObjects = cOtherProgressObjects;
 
     return Progress::init(
@@ -121 +113 @@
                           bstrDescription,
                           fCancelable,
-                          1 + cOtherProgressObjectOperations /* cOperations */,
+                          1 + cOtherProgressObjects + 1 /* cOperations */,
                           uTotalOperationsWeight,
                           bstrFirstOperationDescription,
                           uFirstOperationWeight,
-                          NULL);
+                          pId);
 }
 
@@ -131 +123 @@
 {
     uninit();
-    mfMultiOperation = false;
-    muOtherProgressStartWeight    = 0;
-    muOtherProgressWeight         = 0;
-    muOtherProgressStartOperation = 0;
+    miCurOtherProgressObject = 0;
 }
 
@@ -186 +175 @@
 /** Just a wrapper so we can automatically do the handover before setting
  *  the result locally. */
-bool ProgressProxy::notifyPointOfNoReturn(void)
+bool    ProgressProxy::notifyPointOfNoReturn(void)
 {
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
@@ -199 +188 @@
  * @returns false if failed/canceled, true if not.
  * @param   pOtherProgress      The other progress object. Must not be NULL.
- */
-bool ProgressProxy::setOtherProgressObject(IProgress *pOtherProgress)
-{
-    LogFlowThisFunc(("setOtherProgressObject: %p\n", pOtherProgress));
+ * @param   uOperationWeight    The weight of this operation.  (The description
+ *                              is taken from the other progress object.)
+ */
+bool ProgressProxy::setOtherProgressObject(IProgress *pOtherProgress, ULONG uOperationWeight)
+{
+    LogFlowThisFunc(("setOtherProgressObject: %p %u\n", pOtherProgress, uOperationWeight));
     ComPtr<IProgress> ptrOtherProgress = pOtherProgress;
 
-    /*
-     * Query information from the other progress object before we grab the
-     * lock.
-     */
-    ULONG cOperations;
-    HRESULT hrc = pOtherProgress->COMGETTER(OperationCount)(&cOperations);
-    if (FAILED(hrc))
-        cOperations = 1;
-
-    Bstr bstrOperationDescription;
-    hrc = pOtherProgress->COMGETTER(Description)(bstrOperationDescription.asOutParam());
+    /* Get the description first. */
+    Bstr    bstrOperationDescription;
+    HRESULT hrc = pOtherProgress->COMGETTER(Description)(bstrOperationDescription.asOutParam());
     if (FAILED(hrc))
         bstrOperationDescription = "oops";
 
-
-    /*
-     * Take the lock and check for cancelation, cancel the other object if
-     * we've been canceled already.
-     */
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
+    /* Do the hand over from any previous progress object. */
+    clearOtherProgressObjectInternal(false /*fEarly*/);
     BOOL fCompletedOrCanceled = mCompleted || mCanceled;
     if (!fCompletedOrCanceled)
     {
-        /*
-         * Advance to the next object and operation. If the other object has
-         * more operations than anticipated, adjust our internal count.
-         */
+        /* Advance to the next object and operation, checking for cancelation
+           and completion right away to be on the safe side. */
+        Assert(miCurOtherProgressObject < mcOtherProgressObjects);
         mptrOtherProgress = ptrOtherProgress;
-        mfMultiOperation  = cOperations > 1;
-
-        muOtherProgressStartWeight = m_ulOperationsCompletedWeight + m_ulCurrentOperationWeight;
-        muOtherProgressWeight      = m_ulTotalOperationsWeight - muOtherProgressStartWeight;
-        Progress::SetNextOperation(bstrOperationDescription, muOtherProgressWeight);
-
-        muOtherProgressStartOperation = m_ulCurrentOperation;
-        m_cOperations = cOperations + m_ulCurrentOperation;
-
-        /*
-         * Check for cancelation and completion.
-         */
+
+        Progress::SetNextOperation(bstrOperationDescription, uOperationWeight);
+
         BOOL f;
         hrc = ptrOtherProgress->COMGETTER(Completed)(&f);
@@ -263 +233 @@
         else
         {
-            /*
-             * Finally, mirror the cancelable property.
-             * Note! Note necessary if we do passthru!
-             */
+            /* Mirror the cancelable property. */
             if (mCancelable)
             {
@@ -290 +257 @@
 }
 
+/**
+ * Clears the last other progress objects.
+ *
+ * @returns false if failed/canceled, true if not.
+ * @param   pszLastOperationDescription     The description of the final bit.
+ * @param   uLastOperationWeight            The weight of the final bit.
+ */
+bool ProgressProxy::clearOtherProgressObject(const char *pszLastOperationDescription, ULONG uLastOperationWeight)
+{
+    LogFlowThisFunc(("%p %u\n", pszLastOperationDescription, uLastOperationWeight));
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+    clearOtherProgressObjectInternal(false /* fEarly */);
+
+    /* Advance to the next operation if applicable. */
+    bool fCompletedOrCanceled = mCompleted || mCanceled;
+    if (!fCompletedOrCanceled)
+        Progress::SetNextOperation(Bstr(pszLastOperationDescription), uLastOperationWeight);
+
+    LogFlowThisFunc(("Returns %RTbool\n", !fCompletedOrCanceled));
+    return !fCompletedOrCanceled;
+}
+
 // Internal methods.
 ////////////////////////////////////////////////////////////////////////////////
@@ -295 +285 @@
 
 /**
- * Clear the other progress object reference, first copying over its state.
- *
- * This is used internally when completion is signalled one way or another.
+ * Internal version of clearOtherProgressObject that doesn't advance to the next
+ * operation.
+ *
+ * This is used both by clearOtherProgressObject as well as a number of places
+ * where we automatically do the hand over because of failure/completion.
  *
  * @param   fEarly          Early clearing or not.
@@ -303 +295 @@
 void ProgressProxy::clearOtherProgressObjectInternal(bool fEarly)
 {
-    if (mptrOtherProgress.isNotNull())
+    if (!mptrOtherProgress.isNull())
     {
         ComPtr<IProgress> ptrOtherProgress = mptrOtherProgress;
         mptrOtherProgress.setNull();
         copyProgressInfo(ptrOtherProgress, fEarly);
+
+        miCurOtherProgressObject++;
+        Assert(miCurOtherProgressObject <= mcOtherProgressObjects);
     }
 }
@@ -418 +413 @@
 ////////////////////////////////////////////////////////////////////////////////
 
-STDMETHODIMP ProgressProxy::COMGETTER(Cancelable)(BOOL *aCancelable)
-{
-    CheckComArgOutPointerValid(aCancelable);
+STDMETHODIMP ProgressProxy::COMGETTER(Percent)(ULONG *aPercent)
+{
+#if 0
+    CheckComArgOutPointerValid(aPercent);
 
     AutoCaller autoCaller(this);
     HRESULT hrc = autoCaller.rc();
-    if (SUCCEEDED(hrc))
+    if (SUCCEEDED(rc))
     {
         AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
-        /* ASSUME: The cancelable property can only change to FALSE. */
-        if (!mCancelable || mptrOtherProgress.isNull())
-            *aCancelable = mCancelable;
-        else
-        {
-            hrc = mptrOtherProgress->COMGETTER(Cancelable)(aCancelable);
-            if (SUCCEEDED(hrc) && !*aCancelable)
-            {
-                LogFlowThisFunc(("point-of-no-return reached\n"));
-                mCancelable = FALSE;
-            }
-        }
-    }
-    return hrc;
-}
-
-STDMETHODIMP ProgressProxy::COMGETTER(Percent)(ULONG *aPercent)
-{
-    CheckComArgOutPointerValid(aPercent);
-
-    AutoCaller autoCaller(this);
-    HRESULT hrc = autoCaller.rc();
-    if (SUCCEEDED(hrc))
-    {
-        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-        if (mptrOtherProgress.isNotNull())
+        if (mptrOtherProgress.isNull())
             hrc = Progress::COMGETTER(Percent)(aPercent);
         else
         {
-            /*
-             * Get the overall percent of the other object and adjust it with
-             * the weighting given to the period before proxying started.
-             */
             ULONG uPct;
             hrc = mptrOtherProgress->COMGETTER(Percent)(&uPct);
-            if (SUCCEEDED(hrc))
-            {
-                double rdPercent = ((double)uPct / 100 * muOtherProgressWeight + muOtherProgressStartWeight)
-                                 / m_ulTotalOperationsWeight * 100;
-                *aPercent = RT_MIN((ULONG)rdPercent, 99); /* mptrOtherProgress is cleared when its completed, so we can never return 100%. */
-            }
+            ....
         }
     }
     return hrc;
-}
-
-STDMETHODIMP ProgressProxy::COMGETTER(TimeRemaining)(LONG *aTimeRemaining)
-{
-    CheckComArgOutPointerValid(aTimeRemaining);
-
-    AutoCaller autoCaller(this);
-    HRESULT hrc = autoCaller.rc();
-    if (SUCCEEDED(hrc))
-    {
-        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-        if (mptrOtherProgress.isNotNull())
-            hrc = Progress::COMGETTER(TimeRemaining)(aTimeRemaining);
-        else
-            hrc = mptrOtherProgress->COMGETTER(TimeRemaining)(aTimeRemaining);
-    }
-    return hrc;
+#else
+    return Progress::COMGETTER(Percent)(aPercent);
+#endif
 }
 
 STDMETHODIMP ProgressProxy::COMGETTER(Completed)(BOOL *aCompleted)
 {
-    /* Not proxied since we EXPECT a normal completion notification call. */
+    /* Not proxied since we EXPECT a hand back call. */
     return Progress::COMGETTER(Completed)(aCompleted);
 }
@@ -512 +458 @@
         if (   SUCCEEDED(hrc)
             && !*aCanceled
-            && mptrOtherProgress.isNotNull()
-            && mCancelable)
+            && !mptrOtherProgress.isNull())
         {
             hrc = mptrOtherProgress->COMGETTER(Canceled)(aCanceled);
             if (SUCCEEDED(hrc) && *aCanceled)
-                /* This will not complete the object, only mark it as canceled. */
-                clearOtherProgressObjectInternal(false /*fEarly*/);
+                clearOtherProgressObjectInternal(true /*fEarly*/);
         }
     }
@@ -526 +470 @@
 STDMETHODIMP ProgressProxy::COMGETTER(ResultCode)(LONG *aResultCode)
 {
-    /* Not proxied since we EXPECT a normal completion notification call. */
+    /* Not proxied yet since we EXPECT a hand back call. */
     return Progress::COMGETTER(ResultCode)(aResultCode);
 }
@@ -532 +476 @@
 STDMETHODIMP ProgressProxy::COMGETTER(ErrorInfo)(IVirtualBoxErrorInfo **aErrorInfo)
 {
-    /* Not proxied since we EXPECT a normal completion notification call. */
+    /* Not proxied yet since we EXPECT a hand back call. */
     return Progress::COMGETTER(ErrorInfo)(aErrorInfo);
 }
 
-STDMETHODIMP ProgressProxy::COMGETTER(Operation)(ULONG *aOperation)
-{
-    CheckComArgOutPointerValid(aOperation);
-
-    AutoCaller autoCaller(this);
-    HRESULT hrc = autoCaller.rc();
-    if (SUCCEEDED(hrc))
-    {
-        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-        if (mptrOtherProgress.isNull())
-            hrc =  Progress::COMGETTER(Operation)(aOperation);
-        else
-        {
-            ULONG uCurOtherOperation;
-            hrc = mptrOtherProgress->COMGETTER(Operation)(&uCurOtherOperation);
-            if (SUCCEEDED(hrc))
-                *aOperation = uCurOtherOperation + muOtherProgressStartOperation;
-        }
-    }
-    return hrc;
-}
-
-STDMETHODIMP ProgressProxy::COMGETTER(OperationDescription)(BSTR *aOperationDescription)
-{
-    CheckComArgOutPointerValid(aOperationDescription);
-
-    AutoCaller autoCaller(this);
-    HRESULT hrc = autoCaller.rc();
-    if (SUCCEEDED(hrc))
-    {
-        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-        if (mptrOtherProgress.isNull() || !mfMultiOperation)
-            hrc = Progress::COMGETTER(OperationDescription)(aOperationDescription);
-        else
-            hrc = mptrOtherProgress->COMGETTER(OperationDescription)(aOperationDescription);
-    }
-    return hrc;
-}
-
 STDMETHODIMP ProgressProxy::COMGETTER(OperationPercent)(ULONG *aOperationPercent)
 {
-    CheckComArgOutPointerValid(aOperationPercent);
-
-    AutoCaller autoCaller(this);
-    HRESULT hrc = autoCaller.rc();
-    if (SUCCEEDED(hrc))
-    {
-        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-        if (mptrOtherProgress.isNull() || !mfMultiOperation)
-            hrc = Progress::COMGETTER(OperationPercent)(aOperationPercent);
-        else
-            hrc = mptrOtherProgress->COMGETTER(OperationPercent)(aOperationPercent);
-    }
-    return hrc;
+    /* Not proxied, should be proxied later on. */
+    return Progress::COMGETTER(OperationPercent)(aOperationPercent);
 }
 
@@ -618 +512 @@
     LogFlowThisFunc(("aTimeout=%d\n", aTimeout));
 
-    /* No need to wait on the proxied object for these since we'll get the
-       normal completion notifications. */
+    /* For now we'll always block locally. */
     hrc = Progress::WaitForCompletion(aTimeout);
 
@@ -628 +521 @@
 STDMETHODIMP ProgressProxy::WaitForOperationCompletion(ULONG aOperation, LONG aTimeout)
 {
+    HRESULT hrc;
     LogFlowThisFuncEnter();
     LogFlowThisFunc(("aOperation=%d aTimeout=%d\n", aOperation, aTimeout));
 
-    AutoCaller autoCaller(this);
-    HRESULT hrc = autoCaller.rc();
-    if (SUCCEEDED(hrc))
-    {
-        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-        CheckComArgExpr(aOperation, aOperation < m_cOperations);
-
-        /*
-         * Check if we can wait locally.
-         */
-        if (   aOperation + 1 == m_cOperations /* final operation */
-            || mptrOtherProgress.isNull())
-        {
-            /* ASSUMES that Progress::WaitForOperationCompletion is using
-               AutoWriteLock::leave() as it saves us from duplicating the code! */
-            hrc = Progress::WaitForOperationCompletion(aOperation, aTimeout);
-        }
-        else
-        {
-            LogFlowThisFunc(("calling the other object...\n"));
-            ComPtr<IProgress> ptrOtherProgress = mptrOtherProgress;
-            alock.release();
-
-            hrc = ptrOtherProgress->WaitForOperationCompletion(aOperation, aTimeout);
-        }
-    }
+    /* For now we'll always block locally. Later though, we could consider
+       blocking remotely when we can. */
+    hrc = Progress::WaitForOperationCompletion(aOperation, aTimeout);
 
     LogFlowThisFuncLeave();
@@ -671 +541 @@
     {
         AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-        if (mptrOtherProgress.isNull() || !mCancelable)
-            hrc = Progress::Cancel();
+        if (mCancelable)
+        {
+            if (!mptrOtherProgress.isNull())
+            {
+                hrc = mptrOtherProgress->Cancel();
+                if (SUCCEEDED(hrc))
+                {
+                    if (m_pfnCancelCallback)
+                        m_pfnCancelCallback(m_pvCancelUserArg);
+                    clearOtherProgressObjectInternal(true /*fEarly*/);
+                }
+            }
+            else
+                hrc = Progress::Cancel();
+        }
         else
-        {
-            hrc = mptrOtherProgress->Cancel();
-            if (SUCCEEDED(hrc))
-                clearOtherProgressObjectInternal(false /*fEarly*/);
-        }
+            hrc = setError(E_FAIL, tr("Operation cannot be canceled"));
     }