Index: /trunk/src/VBox/Main/MachineImpl.cpp =================================================================== --- /trunk/src/VBox/Main/MachineImpl.cpp (revision 29936) +++ /trunk/src/VBox/Main/MachineImpl.cpp (revision 29937) @@ -9841,5 +9841,5 @@ if (!mData->mSession.mProgress.isNull()) - mData->mSession.mProgress->setOtherProgressObject(aProgress, 7); + mData->mSession.mProgress->setOtherProgressObject(aProgress); return S_OK; @@ -9863,5 +9863,4 @@ if (mData->mSession.mProgress) { - mData->mSession.mProgress->clearOtherProgressObject(tr("Finalizing"), 1); mData->mSession.mProgress->notifyComplete((HRESULT)iResult); mData->mSession.mProgress.setNull(); Index: /trunk/src/VBox/Main/ProgressProxyImpl.cpp =================================================================== --- /trunk/src/VBox/Main/ProgressProxyImpl.cpp (revision 29936) +++ /trunk/src/VBox/Main/ProgressProxyImpl.cpp (revision 29937) @@ -46,6 +46,8 @@ HRESULT ProgressProxy::FinalConstruct() { - mcOtherProgressObjects = 1; - miCurOtherProgressObject = 0; + mfMultiOperation = false; + muOtherProgressStartWeight = 0; + muOtherProgressWeight = 0; + muOtherProgressStartOperation = 0; HRESULT rc = Progress::FinalConstruct(); @@ -66,6 +68,8 @@ BOOL fCancelable) { - miCurOtherProgressObject = 0; - mcOtherProgressObjects = 0; + mfMultiOperation = false; + muOtherProgressStartWeight = 1; + muOtherProgressWeight = 1; + muOtherProgressStartOperation = 1; return Progress::init( @@ -84,9 +88,12 @@ /** - * 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. + * 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! */ HRESULT ProgressProxy::init( @@ -97,12 +104,13 @@ CBSTR bstrDescription, BOOL fCancelable, - ULONG cOtherProgressObjects, ULONG uTotalOperationsWeight, CBSTR bstrFirstOperationDescription, ULONG uFirstOperationWeight, - OUT_GUID pId) -{ - miCurOtherProgressObject = 0; - mcOtherProgressObjects = cOtherProgressObjects; + ULONG cOtherProgressObjectOperations) +{ + mfMultiOperation = false; + muOtherProgressStartWeight = uFirstOperationWeight; + muOtherProgressWeight = uTotalOperationsWeight - uFirstOperationWeight; + muOtherProgressStartOperation = 1; return Progress::init( @@ -113,9 +121,9 @@ bstrDescription, fCancelable, - 1 + cOtherProgressObjects + 1 /* cOperations */, + 1 + cOtherProgressObjectOperations /* cOperations */, uTotalOperationsWeight, bstrFirstOperationDescription, uFirstOperationWeight, - pId); + NULL); } @@ -123,5 +131,8 @@ { uninit(); - miCurOtherProgressObject = 0; + mfMultiOperation = false; + muOtherProgressStartWeight = 0; + muOtherProgressWeight = 0; + muOtherProgressStartOperation = 0; } @@ -136,13 +147,4 @@ // Public methods //////////////////////////////////////////////////////////////////////////////// - -/** Just a wrapper so we can automatically do the handover before setting - * the result locally. */ -HRESULT ProgressProxy::setResultCode(HRESULT aResultCode) -{ - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - clearOtherProgressObjectInternal(true /* fEarly */); - return Progress::setResultCode(aResultCode); -} /** Just a wrapper so we can automatically do the handover before setting @@ -152,5 +154,8 @@ AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); clearOtherProgressObjectInternal(true /* fEarly */); - return Progress::notifyComplete(aResultCode); + HRESULT hrc = S_OK; + if (!mCompleted) + hrc = Progress::notifyComplete(aResultCode); + return hrc; } @@ -166,18 +171,13 @@ clearOtherProgressObjectInternal(true /* fEarly */); - va_list va; - va_start(va, aText); - HRESULT hrc = Progress::notifyCompleteV(aResultCode, aIID, aComponent, aText, va); - va_end(va); - return hrc; -} - -/** Just a wrapper so we can automatically do the handover before setting - * the result locally. */ -bool ProgressProxy::notifyPointOfNoReturn(void) -{ - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - clearOtherProgressObjectInternal(true /* fEarly */); - return Progress::notifyPointOfNoReturn(); + HRESULT hrc = S_OK; + if (!mCompleted) + { + va_list va; + va_start(va, aText); + HRESULT hrc = Progress::notifyCompleteV(aResultCode, aIID, aComponent, aText, va); + va_end(va); + } + return hrc; } @@ -188,32 +188,51 @@ * @returns false if failed/canceled, true if not. * @param pOtherProgress The other progress object. Must not be NULL. - * @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)); +bool ProgressProxy::setOtherProgressObject(IProgress *pOtherProgress) +{ + LogFlowThisFunc(("setOtherProgressObject: %p\n", pOtherProgress)); ComPtr ptrOtherProgress = pOtherProgress; - /* Get the description first. */ - Bstr bstrOperationDescription; - HRESULT hrc = pOtherProgress->COMGETTER(Description)(bstrOperationDescription.asOutParam()); + /* + * 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()); 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, checking for cancelation - and completion right away to be on the safe side. */ - Assert(miCurOtherProgressObject < mcOtherProgressObjects); + /* + * Advance to the next object and operation. If the other object has + * more operations than anticipated, adjust our internal count. + */ mptrOtherProgress = ptrOtherProgress; - - Progress::SetNextOperation(bstrOperationDescription, uOperationWeight); - + 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. + */ BOOL f; hrc = ptrOtherProgress->COMGETTER(Completed)(&f); @@ -233,5 +252,8 @@ else { - /* Mirror the cancelable property. */ + /* + * Finally, mirror the cancelable property. + * Note! Note necessary if we do passthru! + */ if (mCancelable) { @@ -257,27 +279,4 @@ } -/** - * 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. //////////////////////////////////////////////////////////////////////////////// @@ -285,9 +284,7 @@ /** - * 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. + * Clear the other progress object reference, first copying over its state. + * + * This is used internally when completion is signalled one way or another. * * @param fEarly Early clearing or not. @@ -295,12 +292,9 @@ void ProgressProxy::clearOtherProgressObjectInternal(bool fEarly) { - if (!mptrOtherProgress.isNull()) + if (mptrOtherProgress.isNotNull()) { ComPtr ptrOtherProgress = mptrOtherProgress; mptrOtherProgress.setNull(); copyProgressInfo(ptrOtherProgress, fEarly); - - miCurOtherProgressObject++; - Assert(miCurOtherProgressObject <= mcOtherProgressObjects); } } @@ -413,12 +407,37 @@ //////////////////////////////////////////////////////////////////////////////// +STDMETHODIMP ProgressProxy::COMGETTER(Cancelable)(BOOL *aCancelable) +{ + CheckComArgOutPointerValid(aCancelable); + + AutoCaller autoCaller(this); + HRESULT hrc = autoCaller.rc(); + if (SUCCEEDED(hrc)) + { + 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) { -#if 0 CheckComArgOutPointerValid(aPercent); AutoCaller autoCaller(this); HRESULT hrc = autoCaller.rc(); - if (SUCCEEDED(rc)) + if (SUCCEEDED(hrc)) { AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); @@ -428,18 +447,42 @@ 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); - .... - } - } - return hrc; -#else - return Progress::COMGETTER(Percent)(aPercent); -#endif + 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.isNull()) + hrc = Progress::COMGETTER(TimeRemaining)(aTimeRemaining); + else + hrc = mptrOtherProgress->COMGETTER(TimeRemaining)(aTimeRemaining); + } + return hrc; } STDMETHODIMP ProgressProxy::COMGETTER(Completed)(BOOL *aCompleted) { - /* Not proxied since we EXPECT a hand back call. */ + /* Not proxied since we EXPECT a normal completion notification call. */ return Progress::COMGETTER(Completed)(aCompleted); } @@ -458,9 +501,11 @@ if ( SUCCEEDED(hrc) && !*aCanceled - && !mptrOtherProgress.isNull()) + && mptrOtherProgress.isNotNull() + && mCancelable) { hrc = mptrOtherProgress->COMGETTER(Canceled)(aCanceled); if (SUCCEEDED(hrc) && *aCanceled) - clearOtherProgressObjectInternal(true /*fEarly*/); + /* This will not complete the object, only mark it as canceled. */ + clearOtherProgressObjectInternal(false /*fEarly*/); } } @@ -470,5 +515,5 @@ STDMETHODIMP ProgressProxy::COMGETTER(ResultCode)(LONG *aResultCode) { - /* Not proxied yet since we EXPECT a hand back call. */ + /* Not proxied since we EXPECT a normal completion notification call. */ return Progress::COMGETTER(ResultCode)(aResultCode); } @@ -476,12 +521,62 @@ STDMETHODIMP ProgressProxy::COMGETTER(ErrorInfo)(IVirtualBoxErrorInfo **aErrorInfo) { - /* Not proxied yet since we EXPECT a hand back call. */ + /* Not proxied since we EXPECT a normal completion notification 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) { - /* Not proxied, should be proxied later on. */ - return Progress::COMGETTER(OperationPercent)(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; } @@ -512,5 +607,6 @@ LogFlowThisFunc(("aTimeout=%d\n", aTimeout)); - /* For now we'll always block locally. */ + /* No need to wait on the proxied object for these since we'll get the + normal completion notifications. */ hrc = Progress::WaitForCompletion(aTimeout); @@ -521,11 +617,34 @@ STDMETHODIMP ProgressProxy::WaitForOperationCompletion(ULONG aOperation, LONG aTimeout) { - HRESULT hrc; LogFlowThisFuncEnter(); LogFlowThisFunc(("aOperation=%d aTimeout=%d\n", aOperation, aTimeout)); - /* For now we'll always block locally. Later though, we could consider - blocking remotely when we can. */ - hrc = Progress::WaitForOperationCompletion(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 ptrOtherProgress = mptrOtherProgress; + alock.release(); + + hrc = ptrOtherProgress->WaitForOperationCompletion(aOperation, aTimeout); + } + } LogFlowThisFuncLeave(); @@ -541,21 +660,12 @@ { AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - 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 = setError(E_FAIL, tr("Operation cannot be canceled")); + if (mptrOtherProgress.isNull() || !mCancelable) + hrc = Progress::Cancel(); + else + { + hrc = mptrOtherProgress->Cancel(); + if (SUCCEEDED(hrc)) + clearOtherProgressObjectInternal(false /*fEarly*/); + } } Index: /trunk/src/VBox/Main/VirtualBoxImpl.cpp =================================================================== --- /trunk/src/VBox/Main/VirtualBoxImpl.cpp (revision 29936) +++ /trunk/src/VBox/Main/VirtualBoxImpl.cpp (revision 29937) @@ -1995,16 +1995,21 @@ E_INVALIDARG); + /* get the teleporter enable state for the progress object init. */ + BOOL fTeleporterEnabled; + rc = machine->COMGETTER(TeleporterEnabled)(&fTeleporterEnabled); + if (FAILED(rc)) + return rc; + /* create a progress object */ ComObjPtr progress; progress.createObject(); rc = progress->init(this, - static_cast (machine), + static_cast(machine), Bstr(tr("Spawning session")), TRUE /* aCancelable */, - 1 /* cOtherProgressObjects */, - 10 /* uTotalOperationsWeight */, + fTeleporterEnabled ? 20 : 10 /* uTotalOperationsWeight */, Bstr(tr("Spawning session")), - 3 /* uFirstOperationWeight */, - NULL /* pId */); + 2 /* uFirstOperationWeight */, + fTeleporterEnabled ? 3 : 1 /* cOtherProgressObjectOperations */); if (SUCCEEDED(rc)) { @@ -4521,15 +4526,15 @@ ComPtr cbI; ComPtr cbD; - + cbI = sp; cbD = sp; /** - * Would be like this in ideal world, unfortunately our consumers want to be invoked via IDispatch, + * Would be like this in ideal world, unfortunately our consumers want to be invoked via IDispatch, * thus going the hard way. */ -#if 0 +#if 0 if (cbI != NULL) - { + { HRESULT hrc = handleCallback(cbI); if (hrc == VBOX_E_DONT_CALL_AGAIN) @@ -4544,14 +4549,14 @@ ::VariantClear(&varResult); - ::VariantClear(&arg1); + ::VariantClear(&arg1); ::VariantClear(&arg2); - + VARIANTARG args[] = {arg1, arg2}; DISPPARAMS disp = { args, NULL, sizeof(args)/sizeof(args[0]), 0}; cbD->Invoke(dispid, IID_NULL, - LOCALE_USER_DEFAULT, - DISPATCH_METHOD, - &disp, &varResult, + LOCALE_USER_DEFAULT, + DISPATCH_METHOD, + &disp, &varResult, NULL, NULL); } Index: /trunk/src/VBox/Main/include/ProgressProxyImpl.h =================================================================== --- /trunk/src/VBox/Main/include/ProgressProxyImpl.h (revision 29936) +++ /trunk/src/VBox/Main/include/ProgressProxyImpl.h (revision 29937) @@ -59,17 +59,21 @@ CBSTR bstrDescription, BOOL fCancelable, - ULONG cOtherProgressObjects, ULONG uTotalOperationsWeight, CBSTR bstrFirstOperationDescription, ULONG uFirstOperationWeight, - OUT_GUID pId = NULL); + ULONG cOtherProgressObjectOperations); void uninit(); // IProgress properties + STDMETHOD(COMGETTER(Cancelable))(BOOL *aCancelable); STDMETHOD(COMGETTER(Percent))(ULONG *aPercent); + STDMETHOD(COMGETTER(TimeRemaining))(LONG *aTimeRemaining); STDMETHOD(COMGETTER(Completed))(BOOL *aCompleted); STDMETHOD(COMGETTER(Canceled))(BOOL *aCanceled); STDMETHOD(COMGETTER(ResultCode))(LONG *aResultCode); STDMETHOD(COMGETTER(ErrorInfo))(IVirtualBoxErrorInfo **aErrorInfo); + //STDMETHOD(COMGETTER(OperationCount))(ULONG *aOperationCount); - not necessary + STDMETHOD(COMGETTER(Operation))(ULONG *aOperation); + STDMETHOD(COMGETTER(OperationDescription))(BSTR *aOperationDescription); STDMETHOD(COMGETTER(OperationPercent))(ULONG *aOperationPercent); STDMETHOD(COMSETTER(Timeout))(ULONG aTimeout); @@ -85,5 +89,4 @@ // public methods only for internal purposes - HRESULT setResultCode(HRESULT aResultCode); HRESULT notifyComplete(HRESULT aResultCode); HRESULT notifyComplete(HRESULT aResultCode, @@ -91,7 +94,5 @@ const Bstr &aComponent, const char *aText, ...); - bool notifyPointOfNoReturn(void); - bool setOtherProgressObject(IProgress *pOtherProgress, ULONG uOperationWeight); - bool clearOtherProgressObject(const char *pszLastOperationDescription, ULONG uLastOperationWeight); + bool setOtherProgressObject(IProgress *pOtherProgress); /** For com::SupportErrorInfoImpl. */ @@ -105,8 +106,12 @@ /** The other progress object. This can be NULL. */ ComPtr mptrOtherProgress; - /** The number of other progress objects expected. */ - ULONG mcOtherProgressObjects; - /** The current other progress object. */ - ULONG miCurOtherProgressObject; + /** Set if the other progress object has multiple operations. */ + bool mfMultiOperation; + /** The weight the other progress object started at. */ + ULONG muOtherProgressStartWeight; + /** The weight of other progress object. */ + ULONG muOtherProgressWeight; + /** The operation number the other progress object started at. */ + ULONG muOtherProgressStartOperation; };