Bug 63854 - Calling IProgressService.runInUI while a modal dialog is showing
Summary: Calling IProgressService.runInUI while a modal dialog is showing
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows XP
: P2 major (vote)
Target Milestone: 3.0 RC1   Edit
Assignee: Tod Creasey CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 63605 63607
  Show dependency tree
 
Reported: 2004-05-25 09:58 EDT by Erich Gamma CLA
Modified: 2004-05-31 10:57 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Erich Gamma CLA 2004-05-25 09:58:12 EDT
There is an issue with calling IProgressService.runInUI() while a modal dialog 
is showing. For example, when calling runInUI from a wizard page and the 
operation blocks then the blocked dialog doesn't show up. In the case of the 
wizard page the wizard isn't disabled, a busy cursor is shown, but you can 
still press buttons and cause exceptions easily.

In the current implementation the Blocked dialog is rescheduled until no more 
modal shell is visible: 

BlockedJobsDialog.java
   if (ProgressManagerUtil.rescheduleIfModalShellOpen(this))
       return Status.CANCEL_STATUS;

There must be a good reason for this, but in the case of runInUI() it is 
important that the BlockedDialog shows up immediatly. Disabling the above code 
gives the expected behaviour in the Wizard case, i.e., the blocked dialog 
shows up and blocks the user from further interactions with the wizard.

Therefore the request is to not schedule the blocked dialog in the context of 
runInUI(). Without this support the use of runInUI() will be difficult to 
explain, i.e., don't call this method when a modal dialog is showing.
Comment 1 Tod Creasey CLA 2004-05-25 10:52:55 EDT
The problem with doing it this way is that we cause a regression in the 
general case - the code is there to prevent the opening of a blocked dialog 
when the job is asking for an answer.

Likewise we don't do it in the wizard when running in the UI as it will be 
starved from painting by the UI job. I really don't think we want to do this - 
this was by design in M9 to prevent the appearance of a deadlock with a non-
painting dialog.
Comment 2 Erich Gamma CLA 2004-05-25 12:35:39 EDT
I fully agree we don't want to change the general behaviour.

However, there is the special case when you call runInUI() while there is 
already a modal dialog open. When the only open shell is the one that was open 
when you called runInUI() then it is OK to post the blocked dialog. Is there a 
way to handle this case specially?
Comment 3 John Arthorne CLA 2004-05-25 13:00:36 EDT
The only way I can think of to answer this generally is to allow the client to
specify a parent shell to runInUI. That would allow someone to specifically
parent that progress dialog on the modal wizard dialog.  I think a solution
where we try to "guess" if it is safe to put up a second modal shell is bound to
have exception cases that will cause deadlocks: The operation is blocked asking
the user a question, and the prompt dialog is blocked behind the progress dialog.

Comment 4 Tod Creasey CLA 2004-05-25 13:13:45 EDT
So there is still the painting issue. Even if I open it when it is called from 
the active shell it still won't paint making the UI look unresponsive.

Comment 5 Erich Gamma CLA 2004-05-25 13:23:00 EDT
Re #3: yes we shouldn't guess and passing in the shell explicitly is better.

Re #4: The painting is OK since beginRule() is called with a null progress 
monitor and while waiting you therefore get the EventLoopProgressMonitor. Just 
verified that this works.



Comment 6 Tod Creasey CLA 2004-05-25 13:30:15 EDT
So the question is if we should ask the PMC for an API change or if ou code 
should imply anything. 

If I checked if the context was an IWizardContainer I could handle this case. 
Do we think this is suffecient?
Comment 7 John Arthorne CLA 2004-05-25 14:44:28 EDT
It would be interesting to see if we can come up with a hack to support the
wizard case.

I'm not really keen on adding a parameter to pass the shell here. This would
amount to explicitly encouraging the use of nested modal dialogs (let alone
encouraging running long operations in the UI thread). Longer term, a better
answer is to incorporate the blockage into the wizard dialog rather than opening
a second dialog. Although we failed on this for 3.0, there is hope that we could
come up with some WizardDialogWithBlocking or such to make this happen in the
future. Once we add the API we won't be able to remove it when we arrive at the
"right" answer.
Comment 8 Tod Creasey CLA 2004-05-25 14:56:22 EDT
We haven't failed if you use the new API. It is this migration API that has 
the problem.

If you use the WizardDialog as your progress monitor then it will forward to 
the WorkbenchDialogBlockedHandler which will open the blocked dialog so long 
as you are not running in the UI Thread. If you are in the UIThread we do not 
open it so as to avoid painting problems as it will never update.
 
The reporting of the blockage is already in the dialog. 

Erich why do you want to use runInUI in a wizard anyways?  The old modal 
context code already handles this.
Comment 9 Tod Creasey CLA 2004-05-25 17:06:39 EDT
The issue us that you cannot cancel a job when fork=false in the dialog. We 
are going to look into having the runInUI API use a special progress monitor 
that wrappers the event loop to handle this case.
Comment 10 Tod Creasey CLA 2004-05-26 08:53:26 EDT
I have released a fix based on the discussion Erich, John and I had. I now 
pass an EventLoopProgressMonitor to the beginRule. This one differs from the 
default on in that it will open the blocked jobs dialog regardless as I pass 
it a modal shell or the active workbench window if there isn't one.

I will mark this fixed pending feedback from Erich.
Comment 11 Erich Gamma CLA 2004-05-26 12:56:19 EDT
verified - I've tested the following testcases. 
- running from a WizardPage with the wizard as the runnable context
- running from a custom IRunnableContext
- running with the ApplicationWindow as the runnable context
- running with the IProgressService itself as the runnable context
I didn't find any issues.

Comment 12 Tod Creasey CLA 2004-05-26 13:44:23 EDT
Marking fixed.