[Webkit-unassigned] [Bug 29193] [chromium] Prevent JavaScript busy-loops in unload handlers

[bugzilla-daemon at webkit.org]

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





--- Comment #33 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2009-09-21 13:05:31 PDT ---
(From update of attachment 39590)
I support proceeding with this change.  I think we reached consensus on
webkit-dev to also implement detached Image loads as the big carrot to
accompany this big stick.  I'm fine with making these changes in separate
patches, so long as we get to the carrot soon :-)

Here's some style nits:


> Index: WebCore/bindings/v8/DateExtension.cpp
...
> +DateExtension* DateExtension::s_extension;
> +
> +static const char* s_dateExtensionName = "v8/DateExtension";
> +static const char* s_dateExtensionScript =

nit: WebKit style is to just name static variables the same way you name
local variables, so drop the "s_"


> +v8::Handle<v8::FunctionTemplate> DateExtension::GetNativeFunction(v8::Handle<v8::String> name)
> +{
> +    if (name->Equals(v8::String::New("GiveEnableSleepDetectionFunction"))) {
> +        return v8::FunctionTemplate::New(GiveEnableSleepDetectionFunction);
> +    } else if (name->Equals(v8::String::New("OnSleepDetected"))) {
> +        return v8::FunctionTemplate::New(OnSleepDetected);
> +    }

nit: WebKit style is to not use {}'s around single line statements


> +void DateExtension::weakCallback(v8::Persistent<v8::Value> object, void* param)
> +{
> +    DateExtension* extension = get();
> +    for (FunctionPointerList::iterator i = extension->callEnableSleepDetectionFunctionPointers.begin();
> +         i != extension->callEnableSleepDetectionFunctionPointers.end(); ++i) {
> +        if (*i == object) {
> +            i->Dispose();
> +            extension->callEnableSleepDetectionFunctionPointers.erase(i);
> +            return;
> +        }
> +    }
> +    ASSERT(false);

nit: ASSERT_NOT_REACHED()


> Index: WebCore/bindings/v8/DateExtension.h
...
> +#ifndef DateExtension_h
> +#define DateExtension_h
> +
> +#include <v8.h>
> +
> +#include <list>

nit: Please avoid using STL containers or anything from STL that
allocates memory.  This will matter to Android, which does not have
a complete STL implementation.

maybe you could use WTF's Vector or Deque containers instead?

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