Bug 293098 - [Progress] [Wizards] [JFace] Reporting job progress locally in a wizard, modal jobs
Summary: [Progress] [Wizards] [JFace] Reporting job progress locally in a wizard, moda...
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 enhancement with 2 votes (vote)
Target Milestone: ---   Edit
Assignee: Prakash Rangaraj CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 509006 279385 291631
  Show dependency tree
 
Reported: 2009-10-22 15:26 EDT by Susan McCourt CLA
Modified: 2016-12-16 08:54 EST (History)
13 users (show)

See Also:


Attachments
Patch v01 (16.87 KB, patch)
2010-01-05 11:45 EST, Prakash Rangaraj CLA
no flags Details | Diff
DeferredTree with progress details (31.49 KB, image/png)
2010-01-05 11:45 EST, Prakash Rangaraj CLA
no flags Details
Job progress in the Wizard Dialog (46.91 KB, image/png)
2010-01-05 11:46 EST, Prakash Rangaraj CLA
no flags Details
The Wizard (14.59 KB, application/zip)
2010-01-05 11:47 EST, Prakash Rangaraj CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Susan McCourt CLA 2009-10-22 15:26:17 EDT
I'm opening this bug based on a discussion in bug 279385.  I think that the progress manager (or some Platform UI component) needs to provide a way for clients to run jobs and report progress to an alternate location.  The key point is that this needs to be able to be done in an existing job, not with a special job subclass.  Examples:

1 - We are loading a repository in a wizard and the DeferredTreeContentManager job is running.  The user sees "Pending..." in the wizard's list but the only progress reported is in the status area of the workbench.  The progress view might not even be open and the wizard is modal.  We'd like to be able to report the useful progress (Downloading content.jar 58% done) right there in the wizard.  This is bug 279385.

2 - Some of our operations are run in multiple contexts.  We resolve installs in the background for Help>Check for Updates, but in a ModalContext when we are in a wizard.  Client code has to wrap it differently (ModalContext vs Job) simply because of where the progress needs to be reported.  See ProvisioningOperationRunner for some of this pain.

3 - Headless operation frameworks need a way to supply a generic, headless progress-reporting runnable.  We can't define them as IRunnableWithProgress because we can't assume JFace is there.  We can't use jobs today if we know we want to redirect progress.  (And now we also have IProgressRunnable in the concurrent framework.)

Of course we can work around these problems by copying/hacking the DeferredTreeContentManager, defining a job subclass with extra progress monitors, wrapping monitors, etc.  But we would end up reimplementing stuff like AccumulatingProgressMonitor, ModalContext, etc.  Yuck.  We have almost all the pieces already.

We encourage clients to use jobs, not threads.  So there is lots of API that traffics in jobs already.

Solution:
We need a way to ask the progress component to run a job in a way that reports progress to a UI progress monitor.  We may or may not need it to be modal to the caller.
So code that normally calls
someJob.schedule()

would call
progressManager.run(someJob, monitor) for modal cases and
progressManager.schedule(job, monitor) for cases where it runs like a normal job but we want additional progress reporting.

Note that in both cases, the job would ultimately be run by the job manager, it's just a matter of the progress manager redirecting progress from the progress view to the locally provided progress monitor (or both via wrapping?)

This would allow us to fix the progress reporting issues associated with DeferredTreeContentProvider and simplify our API by defining our units of work as jobs while still allowing clients to run them in a wizard context.

We would also need to generalize the IWorkbenchSiteProgressService handling in DeferredTreeContentManager to include this style of interaction...ie
new DeferredTreeContentManager(treeViewer, myProgressMonitor)

WizardDialog would need to make getProgressMonitor() public so we can do things like:
new DeferredTreeContentManager(treeViewer, getContainer().getProgressMonitor())
and
progressManager.run(myJob, getContainer().getProgressMonitor())
Comment 1 John Arthorne CLA 2009-10-22 17:20:35 EDT
See also bug 276904 about cases where a job gets forked from within a wizard's runnable, causing a second dialog to open. I think this case is important because often there is no direct connection between the wizard's runnable and the code that eventually forks a job - it could be deep down in the bowels of some implementation where a job actually gets forked (as is the case with p2 download jobs).  

This kind of blockage bubbles up the progress chain via IProgressMonitorWithBlocking#setBlocked. The job causing the blockage can actually be determined from here - the IStatus is an IJobStatus that provides a handle to the job. Once you have the job, you can hook into the progress data by adding an IJobProgressManagerListener to ProgressManager. In theory we should be able to report progress on jobs blocking a wizard from completing through these mechanisms.

This bug report also brings to mind IWorkbenchSiteProgressService, which was intended as a context through which a job can be scheduled, to provide progress affordances within that view while the job runs. I could imagine a similar API that allows forking a job within the context of a wizard. Of course this only helps in the "simple" case where the wizards performFinish() is invoking a job directly.
Comment 2 Susan McCourt CLA 2009-10-23 17:54:22 EDT
(In reply to comment #1)
> This bug report also brings to mind IWorkbenchSiteProgressService, which was
> intended as a context through which a job can be scheduled, to provide progress
> affordances within that view while the job runs. I could imagine a similar API
> that allows forking a job within the context of a wizard. Of course this only
> helps in the "simple" case where the wizards performFinish() is invoking a job
> directly.

I'd like to see something more general where you schedule a job through the progress service and provide your desired monitor.  You could also specify whether you want progress reported through the "normal" (dialog and/or progress view) channels (either through a parameter or with an IProgressConstants property constant).  This would allow, for example, someone to put a progress monitor in any dialog and redirect job progress if they wanted to.  

In the particular p2 case of DeferredTreeContentManager, I could see us trying out the progress in the wizard's monitor, but if it didn't play well, we could decide to show a progress bar closer to the tree (or for that matter, we could even write content into/onto the tree) and use the wizard progress bar for the things we do now, like resolving.

As you mention, jobs get run from the bowels of all kinds of operations and if the job progress reporting is not sufficient for the use case, a generic API like this would give apps lots of freedom to solve it.

This would also help with the "there is no progress dialog when a modal dialog is up" problem.  Today, if a job is launched from a modal dialog, there is no progress dialog, even if the user hasn't chosen to send the job to the background.  We work around this in p2 by closing more dialogs than we should have to.  For example, if choose "uninstall" from the installed software page, we close both the installation details and the about dialog so that the progress dialog can be shown (otherwise the user has no feedback besides the status area that anything is happening).  It feels really weird and is only done to work around this problem.  With more flexible progress reporting, we could consider *not* closing the dialog and showing progress in the dialog, letting the user decide if they wanted to close that dialog.
Comment 3 Susan McCourt CLA 2009-10-23 17:58:40 EDT
(In reply to comment #2)
> In the particular p2 case of DeferredTreeContentManager, I could see us trying
> out the progress in the wizard's monitor, but if it didn't play well, we could
> decide to show a progress bar closer to the tree (or for that matter, we could
> even write content into/onto the tree) and use the wizard progress bar for the
> things we do now, like resolving.

While I'm dreaming ;-)
Wouldn't it be cool if there was a Pending Update Adapter that updated the "Pending..." node with progress information?

Instead of Pending... you'd see
Pending...(Downloading http://blah blah blah/content.jar 58%)

If I knew we were going to have this kind of support, I think it would influence the evolution of the p2 API's.  Today we have our own operation hierarchy and if we had the progress monitor problem solved we might be able to just say that our operations are jobs rather than inventing our own interface.
Comment 4 Susan McCourt CLA 2009-10-29 16:53:40 EDT
I wrote up some e4-scoped comments about this problem in 
http://wiki.eclipse.org/E4/EAS/Progress_Service
Comment 5 Boris Bokowski CLA 2009-11-26 16:21:42 EST
Prakash is now responsible for watching bugs in the [Wizards] component area.
Comment 6 Prakash Rangaraj CLA 2009-11-30 04:00:32 EST
Targetting this for M5
Comment 7 Susan McCourt CLA 2009-12-15 13:54:44 EST
Just to prioritize all comments since I was brainstorming.

The most important use case to me in p2 is that I can put a tree backed by DeferredTreeContentManager into a wizard and hook the fetch job's progress into the wizard's progress monitor.  Then users can see what they are waiting on, and with the implementation of bug 287887, they could also stop the fetch.
Comment 8 Prakash Rangaraj CLA 2010-01-05 11:45:01 EST
Created attachment 155343 [details]
Patch v01

Instead of clients wrapping up the IProgressMonitors, we can open up an API like

IProgressService:addProgressMonitorForJob(Job job, IProgressMonitor monitor);

In this way, anyone who is interested in the progress of a job can attach their own monitors.

With a couple of methods exposed from WizardDialog (without adding dependency on Jobs to JFace) and an additional method in the DeferredTreeContentManager, we can achieve this.

Attached is a rough cut of the idea, suggestions are welcome.
Comment 9 Prakash Rangaraj CLA 2010-01-05 11:45:41 EST
Created attachment 155344 [details]
DeferredTree with progress details
Comment 10 Prakash Rangaraj CLA 2010-01-05 11:46:25 EST
Created attachment 155345 [details]
Job progress in the Wizard Dialog
Comment 11 Prakash Rangaraj CLA 2010-01-05 11:47:08 EST
Created attachment 155346 [details]
The Wizard
Comment 12 Susan McCourt CLA 2010-01-05 13:12:54 EST
way cool, I will try these patches out and comment here (it may be tomorrow before I can get to this but it's in the queue)...
Comment 13 Susan McCourt CLA 2010-01-06 19:33:22 EST
I took a look at this.  Very promising!
- I think the ability to add a progress monitor for a job is cool.  
- The example for updating the pending update adapter with progress info is clear.  And I'm already subclassing the content manager and messing around with the pending update adapter so I can do this. (An enhancement would be to have the deferred tree content manager hook into the monitor and automatically update the pending adapter label with progress messages, but that can be done later, and if I really want this I can probably wrap the monitor somehow.)
- The technique for generically starting/stopping an operation in the wizard is cool, and I see how I can tie a job that *I* created to the progress.
- I can also see how to tie a known progress monitor to the tree content manager.  

But I think the public/protected methods are not scoped right.  As it is now you have to subclass both the wizard dialog and the content manager to hook everything up.

The new method "getProgressMonitorForUpdateJob()" implies that the deferred tree content manager should be subclassed to provide the right monitor.  I already have a subclass, so I now supply it with its host dialog and it can use that dialog's monitor.  But that means the content manager needs to trigger the start and stop operations, so those methods would have to be public.  It also means the content manager needs access to the dialog's progress monitor.

The wizard/job example shows it working the other way around - the dialog knows about the job and it triggers the operation, start/stop, and presumably would have to supply its monitor to a content manager subclass.  But a dialog would not know when the content manager job was created or how to get access to it, and it doesn't seem like each component should know about the other one.

So I think:
- DeferredTreeContentManager.getProgressMonitorForUpdateJob() should be protected
- The Wizard Dialog methods startOperation, stopOperation, getProgressMonitor should be public instead of protected
- it would be useful to provide an example of a deferred tree in a dialog that follows this pattern
- it would be useful to show how a deferred tree content manager could wrap the progress monitor so that the label could be updated with the progress messages
Comment 14 Susan McCourt CLA 2010-01-20 16:25:55 EST
fyi, the dependent bugs have been moved to another milestone.  Even if this gets in for M5, we would not have time to adopt.
Comment 15 Markus Duft CLA 2015-09-10 01:55:35 EDT
anybody planning on finishing up this issue? it seems there is already a lot prepared :)