ISaveablePart2.promptToSaveOnClose() invoke on "Save All"

[pcdavid]

The Javadoc for ISaveablePart2.promptToSaveOnClose() indicates that:

This method is only called when the part is closed or when the Workbench is shutting down.

In Sirius we have a case where it is invoked also on "Save All" (not on a plain "Save"), with the following stack (DDiagramEditorImpl is in Sirius, and ):

DDiagramEditorImpl.promptToSaveOnClose() line: 1975	
SaveableHelper.savePart(ISaveablePart2, IWorkbenchWindow, boolean) line: 231	
SaveablesList.preCloseParts(List<IWorkbenchPart>, boolean, boolean, IShellProvider, IWorkbenchWindow, Map<Saveable,Save>) line: 522	
SaveablesList.preCloseParts(List<IWorkbenchPart>, boolean, boolean, IShellProvider, IWorkbenchWindow) line: 484	
WorkbenchWindow$2.saveParts(Collection<MPart>, boolean, boolean, boolean) line: 775	
WorkbenchWindow$2.saveParts(Collection<MPart>, boolean) line: 782	
PartServiceImpl.saveAll(boolean) line: 1470	

In our case this triggers a (redundant) prompt to save dialog when the end user hits "Save All".
I don't understand why WorkbenchWindow$2.saveParts, which is asked just to save our editor invokes SaveablesList.preCloseParts.

[pcdavid]

From our tests, it seems that we did not have the prompt on "Save All" under Eclipse 2019-06, but it is present when using Eclipse 2020-06 (we have not tested the intermediate versions).

[mickaelistria]

Do you think you can write a test-case that reproduces it with a plain text editor? Or at least if you cannot provide a working JUnit Test, some steps to reproduce with plain Eclipse SDK?

[pcdavid]

I'm investigating. I can't reproduce yet outside of Sirius, it might be related to the fact that we can have multiple editors associated to a single in memory session (an EMF ResourceSet), se we have an intermediate class SessionSaveable extends Saveable. I'll update with details when I have more (or close if it ends up being on our side, though this call from a "save" workflow seems to contradict ISaveablePart2.promptToSaveOnClose()'s contract).

[pcdavid]

Still trying to understand what's happening exactly in our case.
I noticed that SaveAllHandler does:

	@Override
	public Object execute(ExecutionEvent event) throws ExecutionException {
		IWorkbenchWindow window = HandlerUtil.getActiveWorkbenchWindowChecked(event);
		IWorkbenchPage page = window.getActivePage();
		if (page != null) {
			((WorkbenchPage) page).saveAllEditors(false, false, true); // A
		}
		EPartService partService = getPartService(window);
		if (partService != null && (partService.getDirtyParts().size() > 0)) {
			partService.saveAll(false);  // B
		}
		return null;
	}

Line A triggers an asynchronous job (launched by org.eclipse.ui.internal.SaveableHelper.doSaveModel(Saveable, IProgressMonitor, IShellProvider, boolean)).

• If the job is finished before we call line B, there are no dirty parts at this point, partService.saveAll(false) is not called and everything is OK.
• If the job is not finished before we hit the second part, partService.getDirtyParts() finds our (still dirty) editor, partService.saveAll(false) is invoked, and we end up with the stack from above where our promptToSaveOnClose() is invoked (wrongly IMO).

I can not reproduce with a simple text editor because it doSave() is done using the
void doSave(IProgressMonitor monitor) variant instead of the IJobRunnable doSave(IProgressMonitor monitor, IShellProvider shellProvider) variant which returns an asynchronous job.

[pcdavid]

The second part of the method (starting from getPartService) has been added by 2c8d34c#diff-13ab9e30514eb8291d9e5e8ef25972f21459342efeb8bcdfaa01c888f50ad41f (gerrit, bugzilla) which appeared first in 2020-06. This is consistent with what I see, i.e. using the exact same version of Sirius, I can reproduce with 2020-06 but not when using 2019-06.