Einav Cohen has submitted this change and it was merged. Change subject: webadmin: Updated SPICE cab download fix (#846341) ...................................................................... webadmin: Updated SPICE cab download fix (#846341) https://bugzilla.redhat.com/846341 Before this patch, this was the situation: class Configurator { protected abstract Event getSpiceVersionFileFetchedEvent(); // Configurator c'tor public Configurator() { ... ... updateSpiceVersion(); } private void updateSpiceVersion() { doSomething(getSpiceVersionFileFetchedEvent()); } } --- class WebAdminConfigurator extends Configurator { public Event spiceVersionFileFetchedEvent = [initialized]; // WebAdminConfigurator c'tor public WebAdminConfigurator(EventBus eventBus) { super(); ... ... } @Override protected Event getSpiceVersionFileFetchedEvent() { return spiceVersionFileFetchedEvent; } } In the WebAdminConfigurator c'tor, there is a call to "super()", i.e., Configurator c'tor, that calls to updateSpiceVersion(), which calls to getSpiceVersionFileFetchedEvent(). getSpiceVersionFileFetchedEvent() is an abstract method in Configurator. Its "implementation" (override) in WebAdminConfigurator returns the spiceVersionFileFetchedEvent member which, theoretically, should be initialized with the "[initialized]" value. However, practically, since the above is happenning during the base class (Configurator) c'tor execution, the WebAdminConfigurator members' initialization hasn't occurred yet, therefore, "spiceVersionFileFetchedEvent" is actually null, and not initialized to "[initialized]" yet. This prevents the "post-fetch-spice-version-file" event from being treated by the relevant event handler, hence the actual server-side SPICE ActiveX cab version is not updated appropriately within the configurator; so the server-side spice-version value practically supplied by the Configurator is the default hard-coded value with which the spiceVersion Configurator member was initialized, i.e. "4.4". So in case the client has a spice ActiveX with a version greater than 4.4, it won't download the SPICE ActiveX cab from the server, since the client thinks that the version of the server-side cab is 4.4, i.e., less than what is currently installed, so no reason to download and upgrade the ActiveX. If the actual version of the server-side cab is greater than 4.4, it can cause a buggy behavior in case client is installed with a version smaller than the actual server-side version: Updated server-side cab should be downloaded to the client since its actual version is larger than the one installed on the client, but due to this bug - it won't happen. E.g.: - reported (buggy) server-side SPICE version: 4.4 - actual server-side SPICE version: 5.0 - SPICE version installed on the client: 4.8. - client asks server for the server-side-SPICE version. - server returns: 4.4 - client compares currently-installed version (4.8) against server-side version (4.4) -> 4.4 is smaller than 4.8 -> client concludes that there is no need to download server-side cab. This is incorrect behavior - actual server-side SPICE version is 5.0, and *should* be downloaded to the client in this case. In this patch, the call to updateSpiceVersion() is done in the WebAdminConfigurator (and UserPortalConfigurator) c'tor, instead of the Configurator c'tor, in a stage in which the spiceVersionFileFetchedEvent member is already initialized; Therefore, once the SPICE version txt file is fetched and read, the version is updated correctly within the configurator, server reports correct version of server-side cab, hence client downloads updated server-side cab when it should. Change-Id: I100e722ed85554451d51e9ddb73f3bb35522684f Signed-off-by: Einav Cohen <ecohen@redhat.com> --- M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Configurator.java M frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/uicommon/UserPortalConfigurator.java M frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/uicommon/WebAdminConfigurator.java 3 files changed, 7 insertions(+), 4 deletions(-) Approvals: Einav Cohen: Verified; Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/7065 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I100e722ed85554451d51e9ddb73f3bb35522684f Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Einav Cohen <ecohen@redhat.com> Gerrit-Reviewer: Daniel Erez <derez@redhat.com> Gerrit-Reviewer: Einav Cohen <ecohen@redhat.com>