Re: [EGIT PATCH 01/11] Add support code to handle plugin property changes

["Tor Arne Vestbø" <torarnv@gmail.com>]

Shawn O. Pearce wrote:
> Tor Arne Vestbø <torarnv@gmail.com> wrote:
>> diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java
>> index fced643..d4a9e8e 100644
>> --- a/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java
>> +++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/Activator.java
>> @@ -44,9 +48,24 @@
>>   * This is a plugin singleton mostly controlling logging.
>>   */
>>  public class Activator extends AbstractUIPlugin {
>> +
>> +	/**
>> +	 *  The one and only instance
>> +	 */
>>  	private static Activator plugin;
> 
> What does this field do?  I don't see it referenced anywhere...

It's the singleton instance, referenced from Activator.getDefault(). I
just documented it :)

>>  	/**
>> +	 * Property listeners for plugin specific events
>> +	 */
>> +	private static List<IPropertyChangeListener> propertyChangeListeners =
>> +		new ArrayList<IPropertyChangeListener>(5);
> 
> None of these list accesses are thread-safe.  Are we certain they
> call will come from a single thread, e.g. the SWT event thread?
> Or do we need to put synchronized protection in here?

The addPropertyChangeListener method is called at startup from the
GitLightweightDecorator constructor, in one of the worker threads, and
same thing with removePropertyChangeListener from dispose(). The
broadcastPropertyChange method is called in the main thread every time
the Git decorator preference page is closes.

This is the sync model used by other Eclipse plugins for keeping track
of propertyChangeListeners, for example in the TeamUIPlugin, that's why
I assumed it was OK. I'm perfectly fine with syncrhronizing it though,
similar to repositoryChangeListeners in GitProjectData?

Tor Arne