On Tue, 27 Jan 2009 18:29:09 +0100, Steve Block <[email protected]> wrote:
> Deepak, some general comments ... > > - Gears doesn't use default values for parameters, so we should either > use a separate method for the case where the BrowsingContext is > supplied, or pass NULL for PIE. > - I think that at least for CabUpdater (the outward-facing API for > this set of classes), we should use a separate Constructor for Opera > which takes the BrowsingContext parameter, rather than modifying the > current constructor. We can then ifdef this on Opera and add a comment > to make it clear that the need for the BrowsingContext is not > fundamental and is only an implementation detail of the Opera > HttpRequest. Andrei, what do you think? I initially changed all methods using an ifdef on Opera but since it was a change that spread across so many files I thought I could minimize the changes needed by adding a default value to the BrowsingContext, this also left the code for pIE unchanged. I agree that there should be a clear comment about BrowsingContext being specific to Opera and ifdef on Opera in CabUpdater is a good idea. > - On Opera, the Cabupdater will be started only when the user first > visits a Gears-enabled page, right? Apologies if you've already > answered this, but what happens if the user navigates away from that > page, thus invalidating the BrowsingContext, before the CabUpdater has > finished using it? Thats correct, in such a case the download of the cab file would fail. Thats the reason I removed the timeout of 2 minutes before connecting to check for the update. We should definitely revisit the implementation of CabUpdater for Opera after this release. Stig will be able to answer better regarding why the browsing context is currently necessary for all requests. > - We should define a constant for the case where "gearsop" is used > multiple times in a file. Agreed. Should I make this change or would you add it when creating the patch? Deepak. > > Steve -- Deepak Prabhakara Developer, Windows Mobile Opera Software +47-96831977
