Community
Participate
Working Groups
AnnotationCodeMiningFilter has a method call isPaintable that (indireclty) calls hasQuickFix(annotation). The isPaintable method is already called in AnnotationCodeMiningFilter#isEmpty what in turn is called in AnnotationCodeMiningProvider#provideCodeMinings. Is this really necessary? If not this would save some unnecessary work.
isPaintable shouldn't call hasQuickFix. hasQuickFix is only necessary to compute whether the annotation has some quick-fix, which is only necessary when drawing the annotation (to hook click handler), or that could even be delayed to the click handler so it's only checked when code mining is clicked on.
See also bug 553044 comment 2 where AnnotationCodeMiningFilter.isPaintable call participates in the deadlock.
(In reply to Mickael Istria from comment #1) > isPaintable shouldn't call hasQuickFix. But it does. See: AnnotationCodeMiningFilter.java#isPaintable(Annotation a) { return annotationAccess.isPaintable(a); //<<<< } annotationAccess.isPaintable() is: public boolean DefaultMarkerAnnotationAccess#isPaintable(Annotation annotation) { if (annotation instanceof IAnnotationPresentation) return true; AnnotationPreference preference= getAnnotationPreference(annotation); if (preference == null) return false; Object type= getType(annotation); String annotationType= (type == null ? null : type.toString()); Image image= getImage(annotation, preference, annotationType); //<<<< return image != null; } getImage() is: private Image getImage(Annotation annotation, AnnotationPreference preference, String annotationType) { ... if (hasQuickFix(annotation)) { //<<<< ... } and finally hasQuickFix is: protected boolean hasQuickFix(Annotation annotation) { if (annotation instanceof IQuickFixableAnnotation) { IQuickFixableAnnotation quickFixableAnnotation= (IQuickFixableAnnotation)annotation; if (!quickFixableAnnotation.isQuickFixableStateSet()) quickFixableAnnotation.setQuickFixable(fQuickAssistAssistant != null && fQuickAssistAssistant.canFix(annotation)); //<<<< return quickFixableAnnotation.isQuickFixable(); } return false; } So you see the the code is in DefaultMarkerAnnotationAccess which is not specific for the AnnotationCodeMiningFilter. Note: I marked interesting lines with "//<<<<"
(In reply to Matthias Becker from comment #3) > (In reply to Mickael Istria from comment #1) > > isPaintable shouldn't call hasQuickFix. > > But it does. See: I trust you it does. My comment was more to interpret "isPaintable must be implement in a way so it doesn't hasQuickFix"
(In reply to Mickael Istria from comment #4) > (In reply to Matthias Becker from comment #3) > > (In reply to Mickael Istria from comment #1) > > > isPaintable shouldn't call hasQuickFix. > > > > But it does. See: > > I trust you it does. My comment was more to interpret "isPaintable must be > implement in a way so it doesn't hasQuickFix" So you propose to change the implementation in DefaultMarkerAnnotationAccess?
(In reply to Matthias Becker from comment #5) > So you propose to change the implementation in DefaultMarkerAnnotationAccess? As long as it's backward compatible and seems worth it, yes.
(In reply to Mickael Istria from comment #6) > (In reply to Matthias Becker from comment #5) > > So you propose to change the implementation in DefaultMarkerAnnotationAccess? > > As long as it's backward compatible and seems worth it, yes. As this class is not specific to the annotations I can't overlook the consequences...