On Thu, Mar 5, 2009 at 1:52 PM, Nicolas Roard <[email protected]> wrote: > On Thu, Mar 5, 2009 at 8:30 PM, Michael Nordman <[email protected]> wrote: >> ======================================================================== >> http://mondrian.corp.google.com/file/10229053///depot/googleclient/gears/opensource/gears/localserver/managed_resource_store_module.cc?a=3 >> File >> //depot/googleclient/gears/opensource/gears/localserver/managed_resource_store_module.cc >> (snapshot 3) >> ------------------------------------ >> Line 204: // in async_task, we check it here. >> This changes the semantics of the method somewhat. Upon return either a check >> should be running or should just have finished. If a caller had set the event >> handlers prior to calling checkForUpdate, they should expect them to be >> called. >> But that won't happen with this change. Also the updateStatus >> lastErrorMessage >> and lastUpdateCheckTime properties will behave differently for updates >> initiated >> thru this method call. > > good point -- but if the only thing that we do if an underlying > asynctask fails is set the > error message, we could maybe do the same here ?
It sets all of the properties mentioned and invokes the onerror callback too sometime after the method returns. Not sure this optimization is worth it. > >> What is the reason for implementing IsOnline at this time? Since chrome >> doesn't >> really have a notion of 'online' we don't bother at all and always consider >> the >> system as online. > > Simply that the call to IsOnline() can be expensive, and if we are > offline there is no > reason to go all the way, so we should bail out early... I think you misunderstood my question. Why implement IsOnline at all, not why call it here. > >> Is there a better way of getting this 'bit' out of the hosting browser than >> groping the 'navigator' object via DOM and script? > > Not really; the browser do all the necessary management of events to know > if the system is online or offline, but only expose this to the DOM. > >> AsyncTasks are often started not in the context of a page, and can run for a >> duration that spans multiple page navigations. Is the techinique you're >> using to >> poke at navigator.online resilient to this? > > Uh... I'm not sure I understand -- AsyncTasks can span multiple page > navigations ? NPN_GetValue takes an NPP instance as a parameter. AsyncTasks can run where there are no NPP instances. > Aren't we destroying them when the plugin get deallocated ?.. > Either way, if the context become unusuable (e.g. if the plugin got > deallocated), > we just return true by default -- also the case if the BrowsingContext > isn't even set. > > -- > Nicolas Roard > Google UK London >
