[webkit-reviews] review denied: [Bug 75111] [Chromium] fast/workers/storage/use-same-database-in-page-and-workers.html times out frequently : [Attachment 130464] v8bindings

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Michael Nordman
<michaeln at google.com>'s request for review:
Bug 75111: [Chromium]
fast/workers/storage/use-same-database-in-page-and-workers.html times out
frequently
https://bugs.webkit.org/show_bug.cgi?id=75111

Attachment 130464: v8bindings
https://bugs.webkit.org/attachment.cgi?id=130464&action=review

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=130464&action=review


> Source/WebCore/ChangeLog:8
> +	   - simplify ActiveDOMCallback, there is no need for it to support

Should be here:
bindings/generic/ActiveDOMCallback.cpp

> Source/WebCore/bindings/js/WorkerScriptController.cpp:165
> +    MutexLocker locker(m_scheduledTerminationMutex);

Why is the mutex needed here?

> Source/WebCore/bindings/v8/WorkerScriptController.cpp:2
> + * Copyright (C) 2012 Google Inc. All rights reserved.

Typically in WebKit the new year is appended.
 2009, 2012

> Source/WebCore/bindings/v8/WorkerScriptController.cpp:56
> +    , m_executionScheduledToTerminate(false)

Actually why add this? Doesn't WorkerRunLoop::terminated() suffice?

> Source/WebCore/dom/ScriptExecutionContext.cpp:95
> +    ,
m_reasonForSuspendingActiveDOMObjects(static_cast<ActiveDOMObject::ReasonForSus
pension>(0))

Why did this get added?

Also if it must be initialized why not pick a value rather than do this
typecasting thing.