[Webkit-unassigned] [Bug 25215] Support loading script for nested workers.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=25215





------- Comment #4 from ap at webkit.org  2009-04-17 08:44 PDT -------
> Who can I ping to consult with this design? Sending an email to webkit-dev?

Yes, an e-mail to webkit-dev sounds fine.

> This is the full implementation of script loading for both workers and nested
> workers, though it is one of steps to support nested workers.

How does it fix a FIXME in
WorkerThreadableLoader::MainThreadBridge::mainThreadCreateLoader? The current
implementation of ThreadableLoader doesn't work in nested workers.

> I just ran prepareChangeLog to create the change log. Do you want me to add
> comments to modified functions in ChangeLog like the following:
>    (WebCore::Worker::didFail): Called when the loading fails.

As you imply, this comment would not be helpful.

The purpose of comments is two-fold. First, they let people not directly
involved with implementation of a feature to follow the progress easily, and
learn new concepts. Second, they help when investigating bugs introduced by a
patch - if a particular change has bad side effects, the person fixing the code
needs to know its primary purpose in order to not defeat it when fixing side
effects.

This patch is not trivial, and it definitely needs a detailed ChangeLog. You
should mention at least the following:
- Worker loading now uses ThreadableLoader instead of CachedScript, because the
latter is not thread safe. Thus, it implements a different set of callbacks;
- terminate() now has a different effect on loading;
- there may be little to say about didFail(), but a comment for
didFailRedirectCheck() can explain how worker loading is supposed to handle
redirects;
- a comment for abortLoading() should explain when it is supposed to be called
- e.g. it's very surprising that it is called from didFail() in your patch;
- anything else that required your special attention, or that could confuse
others reading your patch.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.