> PoolThreadsManager is not used at all, so what it is calling is irrelevant. > The worker threads are handled by opera internally. When the workerpool in > opera creates a new thread it Will register it in the plugin by creating a > OperaWorkerThreadImpl object and then create a new factory to store in the new > js environment. When this factory is created the OperaJsRunner assosiated with
> the thread is also created. I have a couple of comments about the creation of the workerpool objects for Opera. First, the new object is returned to JavaScript as JSPARAM_OBJECT, rather than JSPARAM_MODULE. In all other cases, module objects created by Gears are returned as JSPARAM_MODULE. I'm not sure whether there's anything that currently depends upon this, but behaviour, but I think it would be dangerous in terms of future compatibility to break this pattern. Second and more generally, the fact that we don't use CreateModule and GearsWorkerPool means that new object's API is set in Opera, not in the Gears code. This means that any changes to the WorkerPool API, or to the general behaviour of the Dispatcher and ModuleWrapper classes, would require an update to Opera, as well as to Gears. The same is true of changes to the WorkerPool logic, such as the PoolThreadsManager. One of the reasons we use an autoupdater and a common codebase for all platforms is to provide a quick and easy process for updates. Can we reduce the amount of logic that is implemented inside Opera? I understand that you may have to make calls into Opera to create new worker threads, but can we make use of CreateWorker and GearsWorkerPool as on other platforms? Perhaps Opera could use a different version of PoolThreadsManager to abstract away the platform differences? ======================================================================== http://mondrian.corp.google.com/file/9465156///depot/googleclient/gears/opensource/gears/factory/factory_impl.cc?a=6 File //depot/googleclient/gears/opensource/gears/factory/factory_impl.cc (snapshot 6) ------------------------------------ Line 206: // to be created using a different method than the gears internal ones See general comment above. ------------------------------------ Line 211: context->SetException(STRING16(L"Internal error.")); Probably best to return early. ------------------------------------ Line 218: context->SetReturnValue(JSPARAM_OBJECT, wpo.get()); This relies on the the return at line 269, because object is NULL. It's probably more readable to return early here. ======================================================================== http://mondrian.corp.google.com/file/9465156///depot/googleclient/gears/opensource/gears/factory/factory_np.cc?a=1 File //depot/googleclient/gears/opensource/gears/factory/factory_np.cc (snapshot 1) ------------------------------------ Line 44: factory_impl->SetJsWrapper(factory_wrapper.get()); Can you explain why this is required on Opera? ======================================================================== -- To respond, reply to this email or visit http://mondrian.corp.google.com/9465156
