Bugzilla – Full Text Bug Listing
| Summary: | [Compatibility] Editors occasionally fail to persist state (org.eclipse.ui.internal.emptyEditorTab) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] e4 | Reporter: | Brian de Alwis <bsd> | ||||
| Component: | UI | Assignee: | Remy Suen <remy.suen> | ||||
| Status: | RESOLVED FIXED | QA Contact: | Remy Suen <remy.suen> | ||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | daniel_megert, emoffatt, Mike_Wilson, pwebster, remy.suen | ||||
| Version: | unspecified | Flags: | Mike_Wilson:
pmc_approved+
pwebster: review+ |
||||
| Target Milestone: | 4.1 RC4 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Brian de Alwis
I had wondered if this bug might be related to the exit-not-prompting bug (bug 338221 which caused a the shutdown procedure to execute twice) but this bug just happened to me when switching between workspaces. I got this just now with I20110604-2201 on Windows XP. I had three workbench windows and initated a restart via 'File > Restart'. I20110604-2201 1. Wipe your deltas. 2. Start Eclipse. 3. Open a local file in your workspace. 4. Window > New Window 5. File > Restart 6. Eclipse will return but the editor will not have been restored. 1. Open a file. 2. Make the editor dirty. 3. Window > New Window 4. File > Restart 5. When prompted to save, click 'Cancel'. 6. The second workbench window is closed. All these problems are a generic issue with the implementation and use of the IPresentationEngine's stop() method. Shells are just closed "randomly" with no regard of what happened in the other windows so step 6 happens. Editors do not get persisted properly because the controls simply get disposed so the workbench's internal shutdown code never happens. This bug also means that a view's saveState(IMemento) method will not be called on restart. This also means that editor implementations that implement the IPersistableEditor interface won't be able to save information in a memento. Created attachment 197703 [details] Restart patch v1 I am suggesting we fix this bug for 4.1 GA. IMPACTED USERS: Anybody that uses two or more workbench windows. This problem is not noticeable to anyone with only one workbench window because we detect the single window case and will initiate the regular shutdown sequence. BACKGROUND: The current restart request only goes through the rendering engine's stop() method. The current implementation closes every single shell indiscriminately. This is wrong in many ways. The most obvious problem in question is that the cancellation of this operation does not prevent the other shells from being closed (see comment 4). Arbitrarily closing workbench windows even when the user has chosen to cancel something leads to frustrated users as the layout of their views and editors have now been lost in those other workbench windows. By simply closing the shells without going through our specialized shutdown cycle, editors and views also do not get a chance to clean themselves up so any state that is usually saved in a memento does not get stored on a restart (see comment 5 and comment 6). This also means that the editor inputs do not get persisted so all opened editors on the eagerly closed shells will not get restored on the next startup. This implies that the user has to reopen all of those editors by hand. Not necessarily a direct data loss but an annoyance nonetheless. Please note that dirty parts do get prompted as needing to be saved even in the currently bad code. This behaviour is of course unchanged and will continue to function with the patch. GOALS/BENEFITS: Restart is a common scenario after having installed some plug-ins (which we want consumers to do with 4.1) and making the restart cycle correctly persist their data will ensure that they do not have a sour taste in their mouths when Eclipse comes back up. Having their editors return not persisted properly may give them the impression that their plug-ins were the problem and may send them the message that our compatibility layer implementation is not ready. RELATED BUGS: https://bugs.eclipse.org/bugs/show_bug.cgi?id=312821 RISKS: Minimal. By calling the same close(int, boolean) method that our regular shutdown calls, we can ensure that anyone that uses IWorkbench's API restart() method will have the application shutdown and restart in a way that they expect it to work in 3.x. I was initially hesitant about removing the changes for bug 312821 but to ensure that the shutdown sequence is identical to what happens with a 'File > Exit', I decided that we should remove those extra checks which would guarantee that both shutdown and restart would go through the _exact_ code path. PERFORMANCE IMPACT: Negligible. Restart will be slower (but correct). The speed will be identical to a regular 'File > Exit' shutdown case. (In reply to comment #7) > Created attachment 197703 [details] > Restart patch v1 Paul, please take a look. Makes sense, and makes the code simpler. PW (In reply to comment #7) > Created attachment 197703 [details] > Restart patch v1 Patch released to CVS HEAD. Thanks for reporting the bug, Brian! |