Alexander Wels has uploaded a new change for review.

Change subject: webadmin: revert refresh synchornization
......................................................................

webadmin: revert refresh synchornization

- Revert Change-Id: I8742785cb9b3d890c39859586b03cd53c64b31e5
  As this patch created a bunch of unforseen regressions.
- Properly fixing those issues will take longer than the time
  we have for 3.4

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1066953
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1066489
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1066956

Change-Id: I00c0454852f875d7d2b6734caebda63af47b7eb0
Signed-off-by: Alexander Wels <[email protected]>
---
M 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
M 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridController.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridTimer.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmMonitorModel.java
M 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
8 files changed, 72 insertions(+), 147 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/35/24835/1

diff --git 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
index b466424..ed37e50 100644
--- 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
+++ 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
@@ -19,7 +19,6 @@
 import com.google.gwt.event.logical.shared.ValueChangeHandler;
 import com.google.gwt.event.shared.EventBus;
 import com.google.gwt.event.shared.GwtEvent;
-import com.google.gwt.event.shared.HandlerRegistration;
 import com.google.gwt.event.shared.HasHandlers;
 
 /**
@@ -36,8 +35,8 @@
 
     }
 
-    // Key used to store refresh rate of all data grids
-    private static final String GRID_REFRESH_RATE_KEY = "GridRefreshRate"; 
//$NON-NLS-1$
+    // Prefix for keys used to store refresh rates of individual data grids
+    private static final String GRID_REFRESH_RATE_PREFIX = "GridRefreshRate"; 
//$NON-NLS-1$
 
     private static final Integer DEFAULT_REFRESH_RATE = 
GridTimer.DEFAULT_NORMAL_RATE;
     private static final Integer OUT_OF_FOCUS_REFRESH_RATE = 
Integer.valueOf(60000);
@@ -63,7 +62,6 @@
     private final T refreshPanel;
     private final EventBus eventBus;
     private ManualRefreshCallback manualRefreshCallback;
-    private HandlerRegistration statusUpdateHandlerRegistration;
 
     private GridController controller;
 
@@ -97,22 +95,15 @@
     private void updateController() {
         this.controller = modelProvider.getModel();
 
-        updateTimer();
-    }
+        GridTimer modelTimer = getModelTimer();
+        modelTimer.setRefreshRate(readRefreshRate());
 
-    private void updateTimer() {
-        final GridTimer modelTimer = getModelTimer();
-
-        if (statusUpdateHandlerRegistration != null) {
-            statusUpdateHandlerRegistration.removeHandler();
-        }
-        statusUpdateHandlerRegistration = modelTimer.addValueChangeHandler(new 
ValueChangeHandler<Integer>() {
+        modelTimer.addValueChangeHandler(new ValueChangeHandler<String>() {
             @Override
-            public void onValueChange(ValueChangeEvent<Integer> event) {
-                onRefresh(modelTimer.getTimerRefreshStatus());
+            public void onValueChange(ValueChangeEvent<String> event) {
+                onRefresh(event.getValue());
             }
         });
-        modelTimer.resume();
     }
 
     /**
@@ -165,9 +156,8 @@
     }
 
     public void setCurrentRefreshRate(int newRefreshRate) {
+        getModelTimer().setRefreshRate(newRefreshRate);
         saveRefreshRate(newRefreshRate);
-        getModelTimer().setRefreshRate(readRefreshRate());
-        updateTimer();
     }
 
     public int getCurrentRefreshRate() {
@@ -175,7 +165,7 @@
     }
 
     String getRefreshRateItemKey() {
-        return GRID_REFRESH_RATE_KEY;
+        return GRID_REFRESH_RATE_PREFIX + "_" + controller.getId(); 
//$NON-NLS-1$
     }
 
     void saveRefreshRate(int newRefreshRate) {
diff --git 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java
 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java
index cc699c3..1966cd5 100644
--- 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java
+++ 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java
@@ -177,7 +177,7 @@
 
     private ToggleButton refreshMenuButton;
 
-    private final Image separator;
+    private Image separator;
 
     private final RefreshRateOptionsMenu refreshOptionsMenu;
 
@@ -290,7 +290,11 @@
     }
 
     public void showStatus(String status) {
-        setTitle(status);
+        // for debugging
+        // statusLabel.setText(status);
+        if (refreshManager.getModelTimer().isActive()) {
+            setTitle(status);
+        }
     }
 
     private void createRefreshButton() {
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridController.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridController.java
index 048b83b..d955a32 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridController.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridController.java
@@ -7,13 +7,11 @@
 
     /**
      * Returns the refresh timer used by the Grid.
-     * @return The Grid timer.
      */
     GridTimer getTimer();
 
     /**
      * Returns the controller ID.
-     * @return The controller ID.
      */
     String getId();
 
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridTimer.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridTimer.java
index 81cf77d..8e66fa5 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridTimer.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridTimer.java
@@ -9,9 +9,9 @@
 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.EventBus;
 import com.google.gwt.event.shared.GwtEvent;
 import com.google.gwt.event.shared.HandlerRegistration;
+import com.google.gwt.event.shared.SimpleEventBus;
 import com.google.gwt.user.client.Timer;
 
 /**
@@ -27,7 +27,7 @@
  *     refresh rate). This mode is triggered by the fastForward() method. each 
call reset the cycle
  *     to the start point.
  */
-public abstract class GridTimer extends Timer implements 
HasValueChangeHandlers<Integer> {
+public abstract class GridTimer extends Timer implements 
HasValueChangeHandlers<String> {
 
     private enum RATE {
         FAST {
@@ -111,7 +111,7 @@
 
     private int currentRate = 0;
 
-    private final EventBus eventBus;
+    private final SimpleEventBus eventBus;
 
     private final String name;
 
@@ -125,14 +125,13 @@
 
     private int repetitions;
 
-    public GridTimer(String name, final EventBus eventBus) {
+    public GridTimer(String name) {
         this.name = name;
-        assert(eventBus != null);
-        this.eventBus = eventBus;
+        eventBus = new SimpleEventBus();
     }
 
     @Override
-    public HandlerRegistration 
addValueChangeHandler(ValueChangeHandler<Integer> handler) {
+    public HandlerRegistration 
addValueChangeHandler(ValueChangeHandler<String> handler) {
         return eventBus.addHandler(ValueChangeEvent.getType(), handler);
     }
 
@@ -203,13 +202,14 @@
         logger.fine("GridTimer[" + name + "]: Refresh Rate set to: " + 
interval); //$NON-NLS-1$ //$NON-NLS-2$
         // set the NORMAL interval
         normalInterval = interval;
-        ValueChangeEvent.fire(this, getRefreshRate());
+        start();
     }
 
     public void start() {
         logger.fine("GridTimer[" + name + "].start()"); //$NON-NLS-1$ 
//$NON-NLS-2$
         active = true;
         scheduleRepeating(getRefreshRate());
+        ValueChangeEvent.fire(this, getValue());
     }
 
     public void stop() {
@@ -242,7 +242,7 @@
         return active;
     }
 
-    public String getTimerRefreshStatus() {
+    private String getValue() {
         logger.fine((isActive() ? "Refresh Status: Active(" : "Inactive(") + 
(isPaused() ? "paused)" : "running)") + ":" //$NON-NLS-1$ //$NON-NLS-2$ 
//$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
                 + " Rate: " + rateCycle[currentRate] + "(" + getRefreshRate() 
/ 1000 + " sec)"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$); }
         return 
ConstantsManager.getInstance().getMessages().refreshInterval(getRefreshRate() / 
1000);
@@ -251,7 +251,7 @@
     private void doStop() {
         reset();
         cancel();
-        ValueChangeEvent.fire(this, getRefreshRate());
+        ValueChangeEvent.fire(this, getValue());
     }
 
     private void cycleRate() {
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
index 94ca787..922839f 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
@@ -42,12 +42,6 @@
 import org.ovirt.engine.ui.uicompat.NotifyCollectionChangedEventArgs;
 import org.ovirt.engine.ui.uicompat.PropertyChangedEventArgs;
 
-import com.google.gwt.core.client.Scheduler;
-import com.google.gwt.core.client.Scheduler.ScheduledCommand;
-import com.google.gwt.event.logical.shared.ValueChangeEvent;
-import com.google.gwt.event.logical.shared.ValueChangeHandler;
-import com.google.gwt.event.shared.HandlerRegistration;
-
 /**
  * Represents a list model with ability to fetch items both sync and async.
  */
@@ -60,7 +54,6 @@
     private static final String PAGE_NUMBER_REGEX = "[1-9]+[0-9]*$"; 
//$NON-NLS-1$
 
     private UICommand privateSearchCommand;
-    private HandlerRegistration timerChangeHandler;
 
     public UICommand getSearchCommand()
     {
@@ -303,84 +296,35 @@
         return true;
     }
 
-    /**
-     * Grid refresh timer associated with this list model.
-     */
-    private GridTimer timer;
+    private GridTimer privatetimer;
 
-    /**
-     * Setter for the grid timer.
-     * @param value The new {@code GridTimer}.
-     */
-    private void setTimer(final GridTimer value) {
-        timer = value;
+    public GridTimer gettimer()
+    {
+        return privatetimer;
+    }
+
+    public void settimer(GridTimer value)
+    {
+        privatetimer = value;
     }
 
     @Override
-    public GridTimer getTimer() {
-        if (timer == null && getEventBus() != null) {
-            // The timer doesn't exist yet, and we have an event bus, create 
the timer and pass in the bus.
-            setTimer(new GridTimer(getListName(), getEventBus()) {
+    public GridTimer getTimer()
+    {
+        if (gettimer() == null)
+        {
+            settimer(new GridTimer(getListName()) {
 
                 @Override
                 public void execute() {
-                    // Execute the code, sub classes can override this method 
to get their own code run.
-                    doGridTimerExecute();
+                    logger.fine(SearchableListModel.this.getClass().getName() 
+ ": Executing search"); //$NON-NLS-1$
+                    syncSearch();
                 }
 
             });
-            //Always add a change handler, so we can properly synchronize the 
interval on all GridTimers.
-            replaceTimerChangeHandler();
+            
gettimer().setRefreshRate(getConfigurator().getPollingTimerInterval());
         }
-        return timer;
-    }
-
-    /**
-     * Sub classes can override this method if they need to do something 
different when the timer
-     * expires.
-     */
-    protected void doGridTimerExecute() {
-        logger.fine(SearchableListModel.this.getClass().getName() + ": 
Executing search"); //$NON-NLS-1$
-        syncSearch();
-    }
-
-    /**
-     * Add a {@code ValueChangeHandler} to the timer associated with this 
{@code SearchableListModel}.
-     * The handler is used to update the refresh rate based on changes of 
other timers. So if another timer changes
-     * from lets say 5 seconds to 30 seconds interval. It will fire a {@code 
ValueChangeEvent} which this timer
-     * receives.
-     *
-     * If this timer is currently active (active tab/always active). It will 
stop this timer, change the interval,
-     * and start the timer again. If it is inactive, it will just update the 
interval so that the interval is correct
-     * for when the timer does become active (changing main tabs).
-     */
-    private void addTimerChangeHandler() {
-        timerChangeHandler = timer.addValueChangeHandler(new 
ValueChangeHandler<Integer>() {
-            @Override
-            public void onValueChange(ValueChangeEvent<Integer> event) {
-                int newInterval = event.getValue();
-                if (timer.isActive()) {
-                    //Immediately adjust timer and restart if it was active.
-                    if (newInterval != timer.getRefreshRate()) {
-                        timer.stop();
-                        timer.setRefreshRate(newInterval);
-                        timer.start();
-                    }
-                } else {
-                    //Update the timer interval for inactive timers, so they 
are correct when they become active
-                    timer.setRefreshRate(newInterval);
-                }
-            }
-        });
-    }
-
-    protected void replaceTimerChangeHandler() {
-        if (timerChangeHandler == null) {
-            addTimerChangeHandler();
-        } else {
-            removeTimerChangeHandler();
-            addTimerChangeHandler();
-        }
+        return gettimer();
     }
 
     @Override
@@ -446,20 +390,7 @@
                 onPropertyChanged(new 
PropertyChangedEventArgs(PropertyChangedEventArgs.Args.PROGRESS.toString()));
                 syncSearch();
                 setIsQueryFirstTime(false);
-                if (getTimer() != null) {
-                    //Timer can be null if the event bus hasn't been set yet 
(model hasn't been fully initialized)
-                    startRefresh();
-                } else {
-                    //Defer the start of the timer until after the event bus 
has been added to this model. Then we
-                    //can pass the event bus to the timer and the timer can 
become active.
-                    Scheduler.get().scheduleDeferred(new ScheduledCommand() {
-
-                        @Override
-                        public void execute() {
-                            startRefresh();
-                        }
-                    });
-                }
+                getTimer().start();
             }
             else
             {
@@ -468,21 +399,15 @@
         }
     }
 
-    private void startRefresh() {
-        if (getTimer() != null) {
-            getTimer().start();
-        }
-    }
-
     public void forceRefresh()
     {
-        stopRefresh();
+        getTimer().stop();
         setIsQueryFirstTime(true);
         syncSearch();
 
         if (!getIsTimerDisabled())
         {
-            startRefresh();
+            getTimer().start();
         }
     }
 
@@ -846,17 +771,7 @@
 
     public void stopRefresh()
     {
-        if (getTimer() != null) {
-            //Timer can be null if the event bus hasn't been set yet. If the 
timer is null we can't stop it.
-            getTimer().stop();
-        }
-    }
-
-    protected void removeTimerChangeHandler() {
-        if (timerChangeHandler != null) {
-            timerChangeHandler.removeHandler();
-            timerChangeHandler = null;
-        }
+        getTimer().stop();
     }
 
     @Override
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
index f5c8423..8ab6bd5 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
@@ -20,6 +20,7 @@
 import org.ovirt.engine.ui.uicommonweb.Linq;
 import org.ovirt.engine.ui.uicommonweb.UICommand;
 import org.ovirt.engine.ui.uicommonweb.help.HelpTag;
+import org.ovirt.engine.ui.uicommonweb.models.GridTimer;
 import org.ovirt.engine.ui.uicommonweb.models.ListWithDetailsModel;
 import org.ovirt.engine.ui.uicompat.ConstantsManager;
 import org.ovirt.engine.ui.uicompat.EventArgs;
@@ -28,6 +29,8 @@
 
 public class EventListModel extends ListWithDetailsModel
 {
+    private final GridTimer timer;
+
     private UICommand privateRefreshCommand;
 
     public UICommand getRefreshCommand()
@@ -105,11 +108,16 @@
         getSearchPreviousPageCommand().setIsAvailable(true);
 
         setIsTimerDisabled(true);
-    }
 
-    @Override
-    protected void doGridTimerExecute() {
-        getRefreshCommand().execute();
+        timer = new GridTimer(getListName()) {
+
+            @Override
+            public void execute() {
+                getRefreshCommand().execute();
+            }
+        };
+
+        timer.setRefreshRate(getConfigurator().getPollingTimerInterval());
     }
 
     @Override
@@ -126,6 +134,8 @@
         requestingData = true;
         setItems(new ObservableCollection<AuditLog>());
         setLastEvent(null);
+
+        timer.start();
     }
 
     @Override
@@ -214,6 +224,14 @@
         return getDetailsCommand();
     }
 
+    @Override
+    public void stopRefresh()
+    {
+        super.stopRefresh();
+
+        timer.stop();
+    }
+
     private void updateItems(ArrayList<AuditLog> source)
     {
         if (getItems() == null)
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmMonitorModel.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmMonitorModel.java
index 04f8dfc..42755a4 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmMonitorModel.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmMonitorModel.java
@@ -81,7 +81,7 @@
     {
         if (refreshTimer == null)
         {
-            refreshTimer = new GridTimer("VmMonitorModel", getEventBus()) { 
//$NON-NLS-1$
+            refreshTimer = new GridTimer("VmMonitorModel") { //$NON-NLS-1$
                         @Override
                         public void execute() {
                             refresh();
diff --git 
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
 
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
index 1ac529b..15b2bd9 100644
--- 
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
+++ 
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
@@ -70,9 +70,9 @@
                 }
             }
         }));
-        getModelProvider().getModel().getTimer().addValueChangeHandler(new 
ValueChangeHandler<Integer>() {
+        getModelProvider().getModel().getTimer().addValueChangeHandler(new 
ValueChangeHandler<String>() {
             @Override
-            public void onValueChange(ValueChangeEvent<Integer> event) {
+            public void onValueChange(ValueChangeEvent<String> event) {
                 
getView().setRefreshButtonVisibility(!getModelProvider().getModel().getTimer().isActive());
             }
         });


-- 
To view, visit http://gerrit.ovirt.org/24835
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I00c0454852f875d7d2b6734caebda63af47b7eb0
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Alexander Wels <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to