Bug 371981 - Make IToggleBreakpointsTargetExtension2 usable for clients compiled against older Eclipse versions
Summary: Make IToggleBreakpointsTargetExtension2 usable for clients compiled against o...
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.8   Edit
Hardware: Macintosh Mac OS X
: P3 normal (vote)
Target Milestone: 3.8 M6   Edit
Assignee: Michael Rennie CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2012-02-18 13:34 EST by Christian Georgi CLA
Modified: 2012-02-21 16:55 EST (History)
2 users (show)

See Also:
pawel.1.piech: review+


Attachments
Queries for an adapter instead of the instanceof check (2.68 KB, patch)
2012-02-18 13:39 EST, Christian Georgi CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Georgi CLA 2012-02-18 13:34:04 EST
I would like to use the new IToggleBreakpointsTargetExtension2 in a debug extension that is compiled against a pre-3.8 version of Eclipse.  The idea is, when running on 3.8, to contribute a Java proxy that dynamically implements IToggleBreakpointsTargetExtension2.  However, the current way it is handled in ToggleBreakpointAction doesn't allow this since it assumes the target to directly implement the new interface.

To solve this, the attached patch queries for an adapter to IToggleBreakpointsTargetExtension2 instead of the instanceof check.  A client plug-in could then use the passed-in IToggleBreakpointsTargetExtension2 class in getAdapter() to do the check for the correct platform version.
Comment 1 Christian Georgi CLA 2012-02-18 13:39:35 EST
Created attachment 211236 [details]
Queries for an adapter instead of the instanceof check
Comment 2 Christian Georgi CLA 2012-02-18 13:43:34 EST
Regressions should not occur since the used DebugPlugin.getAdapter() also does the previous instanceof check.
Comment 3 Pawel Piech CLA 2012-02-21 14:09:47 EST
I'm not sure I completely understand your use case.  Do you want to have a plugin in your product that is built against 3.8, which will contribute IToggleBreakpointTargetExtension2, but have the rest of your product built against 3.7?  If so, I don't think we can accommodate you, since maintaining backward compatibility (and having interfaces like"...Extension2") is enough of a pain in the butt.
Comment 4 Michael Rennie CLA 2012-02-21 14:46:02 EST
(In reply to comment #3)
> I'm not sure I completely understand your use case.  Do you want to have a
> plugin in your product that is built against 3.8, which will contribute
> IToggleBreakpointTargetExtension2, but have the rest of your product built
> against 3.7?  If so, I don't think we can accommodate you, since maintaining
> backward compatibility (and having interfaces like"...Extension2") is enough of
> a pain in the butt.

I am assuming Christian means: http://docs.oracle.com/javase/1.5.0/docs/guide/reflection/proxy.html

In that case I think it makes sense to make this small change so that he can create one of the these proxy classes and contribute it as an adapter and have it work. If this is the meaning of the request, we should also extend it to IToggleBreakpointsTargetExtension (for completeness).

I tried his patch and works fine for JDT debug.
Comment 5 Michael Rennie CLA 2012-02-21 15:10:28 EST
I updated the patch a bit to do the same check for IToggleBreakpointsTargetExtension:

http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=f8db67f5e6ef22a2768e856d60a653b792f02385

Thanks for the patch Christian!
Comment 6 Christian Georgi CLA 2012-02-21 16:44:09 EST
That's what I was talking about.  Thanks, guys.
Comment 7 Pawel Piech CLA 2012-02-21 16:55:09 EST
(In reply to comment #4)
> In that case I think it makes sense to make this small change so that he can
> create one of the these proxy classes and contribute it as an adapter and have
> it work...
I have no problem making small changes upon request to make compatibility easier.  Although it seems that if we were to be really consistent we should use getAdapter() for all cases where we check for an extension to a client-implemented interface.  I don't think we want to be going in that direction.