Bugzilla – Full Text Bug Listing
| Summary: | Dispose Page and PageRec in PageBookView.doDestroyPage(IWorkbenchPart, PageRec) | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Tomasz Zarna <tomasz.zarna> | ||||||||
| Component: | UI | Assignee: | Platform-UI-Inbox <Platform-UI-Inbox> | ||||||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | pwebster, remy.suen | ||||||||
| Version: | 3.8 | ||||||||||
| Target Milestone: | --- | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | stalebug | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 367663 | ||||||||||
| Attachments: |
|
||||||||||
(In reply to comment #0) > It would be an API change though, wouldn't it? According to the wiki page, making abstract methods non-abstract is safe. http://wiki.eclipse.org/Evolving_Java-based_APIs_2 Because it was abstract we've basically forced clients to override that method. Changing it would provide a method no current client calls. Although arguably if we did provide a default and allowed it to be overridden or extended, we would offer a default for any new consumers of this class. Remy, how would this fit in with the e4 MPart pattern? Is this something that's easily translated to @PreDestroy? PW Asked for a clarification :-) 1) it won't effect anything currently written. Given that, is it worth it? 2) When Eclipse4 starts to expose true nesting (MPart that contains multiple MParts managed by the framework) will this cause any problems, as the nested parts still care about @PreDestroy not a dispose() method? PW (In reply to comment #3) > 1) it won't effect anything currently written. This may be considered as a good thing, right? :) > Given that, is it worth it? Well, it's an enhancement request to avoid code duplication in any new PageBookView subclasses (like the new HistoryView). It's definitely not a must-have, but I can provide patches to all current implementations of the PBV, no big deal. (In reply to comment #3) > 2) When Eclipse4 starts to expose true nesting (MPart that contains multiple > MParts managed by the framework) will this cause any problems, as the nested > parts still care about @PreDestroy not a dispose() method? In 3.x, the programming model is to write some dispose() method that the framework will call when your object is no longer needed. In 4.x, you would just tag some method with @PreDestroy. I don't feel that this PBV request is related to 4.x. Even if you're a part in a part, you would still have a @PreDestroy method. In 3.x we call dispose(), in 4.x we call @PreDestroy, I don't think the introduction of this API would change how people perceive the two different methods of cleaning up their resources. (In reply to comment #4) > (In reply to comment #3) > > 1) it won't effect anything currently written. > > This may be considered as a good thing, right? :) OK, if that's the point (make it easier going forward) then we can consider this for 4.2 PW Created attachment 208875 [details]
Fix for eclipse.platform.ui v01
Created attachment 208876 [details]
mylyn/context/zip
Created attachment 208877 [details]
Fix for eclipse.platform.text v01
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. If the bug is still relevant, please remove the "stalebug" whiteboard tag. |
Majority if not all subclasses of PageBookView do pretty much the same thing in their implementation #doDestroyPage: protected void doDestroyPage(IWorkbenchPart part, PageRec pageRecord) { IPage page = pageRecord.page; page.dispose(); pageRecord.dispose(); // ... other stuff } I was wondering if we could pull up these 3 lines to PageBookView.doDestroyPage (making it non-abstract) so all subclasses can just call super.doDestroyPage(...) first and proceed with their own stuff. It would be an API change though, wouldn't it?