Use an EventBus to manage star icons

Image widgets that need to track the starred state of a change
register themselves on the EventBus when the widget is attached to
the DOM, and automatically unregister when the widget is removed
from the DOM. This ensures there are no memory leaks in the browser.

Events can be fired on the event bus to update UI widgets with the
new status of the star, enabling the UI to immediately reflect what
the user has done.

Toggle star RPCs are batched and sent in the background, with at most
one star RPC pending at any one time. If the user rapidly clicks a
lot of search results to be starred, these will batch and send as
soon as the first star action is updated. On slower networks this
will run more quickly than starring each change individually. In
the common case of starring a single change, the batching has no
additional overhead

If any RPC fails the pending stars are undone in the UI and an
error is shown to the user. Any outstanding batch is discarded.

Change-Id: I7ea46d2e2ee5c64c0d807677859cfb7d90b8966a

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeCache.java

@@ -38,7 +38,6 @@ public class ChangeCache {
   private Change.Id changeId;
   private ChangeDetailCache detail;
   private ListenableValue<ChangeInfo> info;
-  private StarCache starred;
 
   protected ChangeCache(Change.Id chg) {
     changeId = chg;
@@ -61,11 +60,4 @@ public class ChangeCache {
     }
     return info;
   }
-
-  public StarCache getStarCache() {
-    if (starred == null) {
-      starred = new StarCache(changeId);
-    }
-    return starred;
-  }
 }

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDetailCache.java

@@ -65,6 +65,7 @@ public class ChangeDetailCache extends ListenableValue<ChangeDetail> {
   public static void setChangeDetail(ChangeDetail detail) {
     Change.Id chgId = detail.getChange().getId();
     ChangeCache.get(chgId).getChangeDetailCache().set(detail);
+    StarredChanges.fireChangeStarEvent(chgId, detail.isStarred());
   }
 
   private final Change.Id changeId;

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java

@@ -28,9 +28,9 @@ import com.google.gerrit.common.data.ChangeDetail;
 import com.google.gerrit.common.data.ChangeInfo;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Change.Status;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.Change.Status;
 import com.google.gwt.event.dom.client.ChangeEvent;
 import com.google.gwt.event.dom.client.ChangeHandler;
 import com.google.gwt.event.dom.client.KeyPressEvent;
@@ -42,7 +42,6 @@ import com.google.gwt.user.client.ui.DisclosurePanel;
 import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.Grid;
 import com.google.gwt.user.client.ui.HorizontalPanel;
-import com.google.gwt.user.client.ui.Image;
 import com.google.gwt.user.client.ui.InlineLabel;
 import com.google.gwt.user.client.ui.Label;
 import com.google.gwt.user.client.ui.ListBox;
@@ -60,9 +59,7 @@ public class ChangeScreen extends Screen
   private final Change.Id changeId;
   private final PatchSet.Id openPatchSetId;
   private ChangeDetailCache detailCache;
-  private StarCache starred;
 
-  private Image starChange;
   private ChangeDescriptionBlock descriptionBlock;
   private ApprovalTable approvals;
 
@@ -155,8 +152,6 @@ public class ChangeScreen extends Screen
     detailCache = cache.getChangeDetailCache();
     detailCache.addValueChangeHandler(this);
 
-    starred = cache.getStarCache();
-
     addStyleName(Gerrit.RESOURCES.css().changeScreen());
 
     keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation());
@@ -165,13 +160,13 @@ public class ChangeScreen extends Screen
     keysNavigation.add(new ExpandCollapseDependencySectionKeyCommand(0, 'd', Util.C.expandCollapseDependencies()));
 
     if (Gerrit.isSignedIn()) {
-      keysAction.add(starred.new KeyCommand(0, 's', Util.C.changeTableStar()));
+      StarredChanges.Icon star = StarredChanges.createIcon(changeId, false);
+      star.setStyleName(Gerrit.RESOURCES.css().changeScreenStarIcon());
+      setTitleWest(star);
+
+      keysAction.add(StarredChanges.newKeyCommand(star));
       keysAction.add(new PublishCommentsKeyCommand(0, 'r', Util.C
           .keyPublishComments()));
-
-      starChange = starred.createStar();
-      starChange.setStyleName(Gerrit.RESOURCES.css().changeScreenStarIcon());
-      setTitleWest(starChange);
     }
 
     descriptionBlock = new ChangeDescriptionBlock();

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java

@@ -145,7 +145,7 @@ public class ChangeTable extends NavigationTable<ChangeInfo> {
   protected void onStarClick(final int row) {
     final ChangeInfo c = getRowItem(row);
     if (c != null && Gerrit.isSignedIn()) {
-       ChangeCache.get(c.getId()).getStarCache().toggleStar();
+      ((StarredChanges.Icon) table.getWidget(row, C_STAR)).toggleStar();
     }
   }
 
@@ -198,7 +198,7 @@ public class ChangeTable extends NavigationTable<ChangeInfo> {
     final String idstr = c.getKey().abbreviate();
     table.setWidget(row, C_ARROW, null);
     if (Gerrit.isSignedIn()) {
-      table.setWidget(row, C_STAR, cache.getStarCache().createStar());
+      table.setWidget(row, C_STAR, StarredChanges.createIcon(c.getId(), c.isStarred()));
     }
     table.setWidget(row, C_ID, new TableChangeLink(idstr, c));
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/StarCache.java

@@ -1,139 +0,0 @@
-// Copyright (C) 2012 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.client.changes;
-
-import com.google.gerrit.client.Gerrit;
-import com.google.gerrit.client.rpc.GerritCallback;
-import com.google.gerrit.client.ui.NeedsSignInKeyCommand;
-import com.google.gerrit.common.data.ChangeDetail;
-import com.google.gerrit.common.data.ChangeInfo;
-import com.google.gerrit.common.data.ToggleStarRequest;
-import com.google.gerrit.reviewdb.client.Change;
-
-import com.google.gwt.event.dom.client.ClickEvent;
-import com.google.gwt.event.dom.client.ClickHandler;
-import com.google.gwt.event.dom.client.KeyPressEvent;
-import com.google.gwt.event.logical.shared.HasValueChangeHandlers;
-import com.google.gwt.event.logical.shared.ValueChangeEvent;
-import com.google.gwt.event.logical.shared.ValueChangeHandler;
-import com.google.gwt.event.shared.GwtEvent;
-import com.google.gwt.event.shared.HandlerManager;
-import com.google.gwt.event.shared.HandlerRegistration;
-import com.google.gwt.resources.client.ImageResource;
-import com.google.gwt.user.client.ui.Image;
-import com.google.gwtjsonrpc.common.VoidResult;
-
-public class StarCache implements HasValueChangeHandlers<Boolean> {
-  public class KeyCommand extends NeedsSignInKeyCommand {
-    public KeyCommand(int mask, char key, String help) {
-      super(mask, key, help);
-    }
-
-    @Override
-    public void onKeyPress(final KeyPressEvent event) {
-      StarCache.this.toggleStar();
-    }
-  }
-
-  ChangeCache cache;
-
-  private HandlerManager manager = new HandlerManager(this);
-
-  public StarCache(final Change.Id chg) {
-    cache = ChangeCache.get(chg);
-  }
-
-  public boolean get() {
-    ChangeDetail detail = cache.getChangeDetailCache().get();
-    if (detail != null) {
-      return detail.isStarred();
-    }
-    ChangeInfo info = cache.getChangeInfoCache().get();
-    if (info != null) {
-      return info.isStarred();
-    }
-    return false;
-  }
-
-  public void set(final boolean s) {
-    if (Gerrit.isSignedIn() && s != get()) {
-      final ToggleStarRequest req = new ToggleStarRequest();
-      req.toggle(cache.getChangeId(), s);
-
-      Util.LIST_SVC.toggleStars(req, new GerritCallback<VoidResult>() {
-        public void onSuccess(final VoidResult result) {
-          setStarred(s);
-          fireEvent(new ValueChangeEvent<Boolean>(s){});
-        }
-      });
-    }
-  }
-
-  private void setStarred(final boolean s) {
-    ChangeDetail detail = cache.getChangeDetailCache().get();
-    if (detail != null) {
-      detail.setStarred(s);
-    }
-    ChangeInfo info = cache.getChangeInfoCache().get();
-    if (info != null) {
-      info.setStarred(s);
-    }
-  }
-
-  public void toggleStar() {
-    set(!get());
-  }
-
-  @SuppressWarnings("unchecked")
-  public Image createStar() {
-    final Image star = new Image(getResource());
-    star.setVisible(Gerrit.isSignedIn());
-
-    star.addClickHandler(new ClickHandler() {
-      @Override
-      public void onClick(final ClickEvent event) {
-        StarCache.this.toggleStar();
-      }
-    });
-
-    @SuppressWarnings("rawtypes")
-    ValueChangeHandler starUpdater = new ValueChangeHandler() {
-        @Override
-        public void onValueChange(ValueChangeEvent event) {
-          star.setResource(StarCache.this.getResource());
-        }
-      };
-
-    cache.getChangeDetailCache().addValueChangeHandler(starUpdater);
-    cache.getChangeInfoCache().addValueChangeHandler(starUpdater);
-
-    this.addValueChangeHandler(starUpdater);
-
-    return star;
-  }
-
-  private ImageResource getResource() {
-    return get() ? Gerrit.RESOURCES.starFilled() : Gerrit.RESOURCES.starOpen();
-  }
-
-  public void fireEvent(GwtEvent<?> event) {
-    manager.fireEvent(event);
-  }
-
-  public HandlerRegistration addValueChangeHandler(
-      ValueChangeHandler<Boolean> handler) {
-    return manager.addHandler(ValueChangeEvent.getType(), handler);
-  }
-}

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/StarredChanges.java

@@ -0,0 +1,216 @@
+// Copyright (C) 2012 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.client.changes;
+
+import com.google.gerrit.client.Gerrit;
+import com.google.gerrit.client.rpc.GerritCallback;
+import com.google.gerrit.common.data.ToggleStarRequest;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gwt.event.dom.client.ClickEvent;
+import com.google.gwt.event.dom.client.ClickHandler;
+import com.google.gwt.event.dom.client.KeyPressEvent;
+import com.google.gwt.event.shared.EventBus;
+import com.google.gwt.event.shared.SimpleEventBus;
+import com.google.gwt.resources.client.ImageResource;
+import com.google.gwt.user.client.ui.Image;
+import com.google.gwtexpui.globalkey.client.KeyCommand;
+import com.google.gwtjsonrpc.common.VoidResult;
+import com.google.web.bindery.event.shared.Event;
+import com.google.web.bindery.event.shared.HandlerRegistration;
+
+/** Supports the star icon displayed on changes and tracking the status. */
+public class StarredChanges {
+  private static final EventBus eventBus = new SimpleEventBus();
+  private static final Event.Type<ChangeStarHandler> TYPE =
+      new Event.Type<ChangeStarHandler>();
+
+  /** Handler that can receive notifications of a change's starred status. */
+  public static interface ChangeStarHandler {
+    public void onChangeStar(ChangeStarEvent event);
+  }
+
+  /** Event fired when a star changes status. The new status is reported. */
+  public static class ChangeStarEvent extends Event<ChangeStarHandler> {
+    private boolean starred;
+
+    public ChangeStarEvent(Change.Id source, boolean starred) {
+      setSource(source);
+      this.starred = starred;
+    }
+
+    public boolean isStarred() {
+      return starred;
+    }
+
+    @Override
+    public Type<ChangeStarHandler> getAssociatedType() {
+      return TYPE;
+    }
+
+    @Override
+    protected void dispatch(ChangeStarHandler handler) {
+      handler.onChangeStar(this);
+    }
+  }
+
+  /**
+   * Create a star icon for the given change, and current status. Returns null
+   * if the user is not signed in and cannot support starred changes.
+   */
+  public static Icon createIcon(Change.Id source, boolean starred) {
+    return Gerrit.isSignedIn() ? new Icon(source, starred) : null;
+  }
+
+  /** Make a key command that toggles the star for a change. */
+  public static KeyCommand newKeyCommand(final Icon icon) {
+    return new KeyCommand(0, 's', Util.C.changeTableStar()) {
+      @Override
+      public void onKeyPress(KeyPressEvent event) {
+        icon.toggleStar();
+      }
+    };
+  }
+
+  /** Add a handler to listen for starred status to change. */
+  public static HandlerRegistration addHandler(
+      Change.Id source,
+      ChangeStarHandler handler) {
+    return eventBus.addHandlerToSource(TYPE, source, handler);
+  }
+
+  /**
+   * Broadcast the current starred value of a change to UI widgets. This does
+   * not RPC to the server and does not alter the starred status of a change.
+   */
+  public static void fireChangeStarEvent(Change.Id id, boolean starred) {
+    eventBus.fireEventFromSource(
+        new ChangeStarEvent(id, starred),
+        id);
+  }
+
+  /**
+   * Set the starred status of a change. This method broadcasts to all
+   * interested UI widgets and sends an RPC to the server to record the
+   * updated status.
+   */
+  public static void toggleStar(
+      final Change.Id changeId,
+      final boolean newValue) {
+    if (next == null) {
+      next = new ToggleStarRequest();
+    }
+    next.toggle(changeId, newValue);
+    fireChangeStarEvent(changeId, newValue);
+    if (!busy) {
+      start();
+    }
+  }
+
+  private static ToggleStarRequest next;
+  private static boolean busy;
+
+  private static void start() {
+    final ToggleStarRequest req = next;
+    next = null;
+    busy = true;
+
+    Util.LIST_SVC.toggleStars(req, new GerritCallback<VoidResult>() {
+      @Override
+      public void onSuccess(VoidResult result) {
+        if (next != null) {
+          start();
+        } else {
+          busy = false;
+        }
+      }
+
+      @Override
+      public void onFailure(Throwable caught) {
+        rollback(req);
+        if (next != null) {
+          rollback(next);
+          next = null;
+        }
+        busy = false;
+        super.onFailure(caught);
+      }
+    });
+  }
+
+  private static void rollback(ToggleStarRequest req) {
+    if (req.getAddSet() != null) {
+      for (Change.Id id : req.getAddSet()) {
+        fireChangeStarEvent(id, false);
+      }
+    }
+    if (req.getRemoveSet() != null) {
+      for (Change.Id id : req.getRemoveSet()) {
+        fireChangeStarEvent(id, true);
+      }
+    }
+  }
+
+  public static class Icon extends Image
+      implements ChangeStarHandler, ClickHandler {
+    private final Change.Id changeId;
+    private boolean starred;
+    private HandlerRegistration handler;
+
+    Icon(Change.Id changeId, boolean starred) {
+      super(resource(starred));
+      this.changeId = changeId;
+      this.starred = starred;
+      addClickHandler(this);
+    }
+
+    /**
+     * Toggles the state of the star, as if the user clicked on the image. This
+     * will broadcast the new star status to all interested UI widgets, and RPC
+     * to the server to store the changed value.
+     */
+    public void toggleStar() {
+      StarredChanges.toggleStar(changeId, !starred);
+    }
+
+    @Override
+    protected void onLoad() {
+      handler = StarredChanges.addHandler(changeId, this);
+    }
+
+    @Override
+    protected void onUnload() {
+      handler.removeHandler();
+      handler = null;
+    }
+
+    @Override
+    public void onChangeStar(ChangeStarEvent event) {
+      setResource(resource(event.isStarred()));
+      starred = event.isStarred();
+    }
+
+    @Override
+    public void onClick(ClickEvent event) {
+      toggleStar();
+    }
+
+    private static ImageResource resource(boolean starred) {
+      return starred ? Gerrit.RESOURCES.starFilled() : Gerrit.RESOURCES.starOpen();
+    }
+  }
+
+  private StarredChanges() {
+  }
+}