Vojtech Szocs has posted comments on this change. Change subject: webadmin: fix events not updating ......................................................................
Patch Set 5: Code-Review+1 (2 comments) Some minor comments, otherwise looks good. http://gerrit.ovirt.org/#/c/24409/5/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java: Line 83: } Line 84: }); Line 85: Line 86: updateController(); Line 87: getModelTimer().setRefreshRate(readRefreshRate()); Small thing: getModelTimer().setRefreshRate(readRefreshRate()); appears twice in code, i.e. it's called here (in constructor, right after updateController which calls updateTimer) and also in setCurrentRefreshRate (right before updateTimer). In other words, consider removing above change and doing: private void updateTimer() { // existing code getModelTimer().setRefreshRate(readRefreshRate()); } public void setCurrentRefreshRate(int newRefreshRate) { // existing code // remove line: getModelTimer().setRefreshRate(readRefreshRate()); } Line 88: Line 89: // Add handler to be notified when the application window gains or looses its focus Line 90: eventBus.addHandler(ApplicationFocusChangeEvent.getType(), new ApplicationFocusChangeHandler() { Line 91: @Override http://gerrit.ovirt.org/#/c/24409/5/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/EntityModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/EntityModel.java: Line 191: public final void setEventBus(EventBus eventBus) { Line 192: if (eventBus == null) { Line 193: unregisterHandlers(); Line 194: this.eventBus = null; Line 195: } else if (this.eventBus == null || this.eventBus != eventBus){ IIUC, this is to guard against multiple potential setEventBus calls with same eventBus instance? If yes, just wondering when could such situation happen. Also, I think we don't need "this.eventBus == null" condition in the else clause, because in that else clause "eventBus != null" and for "this.eventBus == null" scenario, "this.eventBus != eventBus" will also return true. In other words, IIUC the "this.eventBus == null" condition part is redundant. Line 196: this.eventBus = eventBus; Line 197: registerHandlers(); Line 198: } Line 199: } -- To view, visit http://gerrit.ovirt.org/24409 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I484dff03a5f90679ccaa6ca0b58a50a2b3083df9 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Einav Cohen <[email protected]> Gerrit-Reviewer: Greg Sheremeta <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
