- Revision
- 142624
- Author
- [email protected]
- Date
- 2013-02-12 07:33:02 -0800 (Tue, 12 Feb 2013)
Log Message
Merge 142565
> [V8] ScheduledAction::m_context can be empty, so we shouldn't
> retrieve an Isolate by using m_context->GetIsolate()
> https://bugs.webkit.org/show_bug.cgi?id=109523
>
> Reviewed by Adam Barth.
>
> Chromium bug: https://code.google.com/p/chromium/issues/detail?id=175307#makechanges
>
> Currently ScheduledAction is retrieving an Isolate by using m_context->GetIsolate().
> This can crash because ScheduledAction::m_context can be empty. Specifically,
> ScheduledAction::m_context is set to ScriptController::currentWorldContext(),
> which can return an empty handle when a frame does not exist. In addition,
> 'if(context.IsEmpty())' in ScheduledAction.cpp implies that it can be empty.
>
> Alternately, we should pass an Isolate explicitly when a ScheduledAction is instantiated.
>
> No tests. The Chromium crash report doesn't provide enough information
> to reproduce the bug.
>
> * bindings/v8/ScheduledAction.cpp:
> (WebCore::ScheduledAction::ScheduledAction):
> (WebCore):
> (WebCore::ScheduledAction::~ScheduledAction):
> * bindings/v8/ScheduledAction.h:
> (ScheduledAction):
> * bindings/v8/custom/V8DOMWindowCustom.cpp:
> (WebCore::WindowSetTimeoutImpl):
> * bindings/v8/custom/V8WorkerContextCustom.cpp:
> (WebCore::SetTimeoutOrInterval):
>
[email protected]
Review URL: https://codereview.chromium.org/12211130
Modified Paths
Diff
Modified: branches/chromium/1410/Source/WebCore/bindings/v8/ScheduledAction.cpp (142623 => 142624)
--- branches/chromium/1410/Source/WebCore/bindings/v8/ScheduledAction.cpp 2013-02-12 15:29:53 UTC (rev 142623)
+++ branches/chromium/1410/Source/WebCore/bindings/v8/ScheduledAction.cpp 2013-02-12 15:33:02 UTC (rev 142624)
@@ -49,21 +49,28 @@
namespace WebCore {
-ScheduledAction::ScheduledAction(v8::Handle<v8::Context> context, v8::Handle<v8::Function> function, int argc, v8::Handle<v8::Value> argv[])
+ScheduledAction::ScheduledAction(v8::Handle<v8::Context> context, v8::Handle<v8::Function> function, int argc, v8::Handle<v8::Value> argv[], v8::Isolate* isolate)
: m_context(context)
, m_function(function)
, m_code(String(), KURL(), TextPosition::belowRangePosition())
+ , m_isolate(isolate)
{
- v8::Isolate* isolate = m_context->GetIsolate();
m_args.reserveCapacity(argc);
for (int i = 0; i < argc; ++i)
- m_args.append(v8::Persistent<v8::Value>::New(isolate, argv[i]));
+ m_args.append(v8::Persistent<v8::Value>::New(m_isolate, argv[i]));
}
+ScheduledAction::ScheduledAction(v8::Handle<v8::Context> context, const String& code, const KURL& url, v8::Isolate* isolate)
+ : m_context(context)
+ , m_code(code, url)
+ , m_isolate(isolate)
+{
+}
+
ScheduledAction::~ScheduledAction()
{
for (size_t i = 0; i < m_args.size(); ++i) {
- m_args[i].Dispose(m_context->GetIsolate());
+ m_args[i].Dispose(m_isolate);
m_args[i].Clear();
}
}
Modified: branches/chromium/1410/Source/WebCore/bindings/v8/ScheduledAction.h (142623 => 142624)
--- branches/chromium/1410/Source/WebCore/bindings/v8/ScheduledAction.h 2013-02-12 15:29:53 UTC (rev 142623)
+++ branches/chromium/1410/Source/WebCore/bindings/v8/ScheduledAction.h 2013-02-12 15:33:02 UTC (rev 142624)
@@ -45,15 +45,10 @@
class ScheduledAction {
public:
- ScheduledAction(v8::Handle<v8::Context>, v8::Handle<v8::Function>, int argc, v8::Handle<v8::Value> argv[]);
-
- ScheduledAction(v8::Handle<v8::Context> context, const String& code, const KURL& url = ""
- : m_context(context)
- , m_code(code, url)
- {
- }
-
+ ScheduledAction(v8::Handle<v8::Context>, v8::Handle<v8::Function>, int argc, v8::Handle<v8::Value> argv[], v8::Isolate*);
+ ScheduledAction(v8::Handle<v8::Context>, const String&, const KURL&, v8::Isolate*);
~ScheduledAction();
+
void execute(ScriptExecutionContext*);
private:
@@ -66,6 +61,7 @@
ScopedPersistent<v8::Function> m_function;
Vector<v8::Persistent<v8::Value> > m_args;
ScriptSourceCode m_code;
+ v8::Isolate* m_isolate;
};
} // namespace WebCore
Modified: branches/chromium/1410/Source/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp (142623 => 142624)
--- branches/chromium/1410/Source/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp 2013-02-12 15:29:53 UTC (rev 142623)
+++ branches/chromium/1410/Source/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp 2013-02-12 15:33:02 UTC (rev 142624)
@@ -121,7 +121,7 @@
// params is passed to action, and released in action's destructor
ASSERT(imp->frame());
- OwnPtr<ScheduledAction> action = "" ScheduledAction(imp->frame()->script()->currentWorldContext(), v8::Handle<v8::Function>::Cast(function), paramCount, params));
+ OwnPtr<ScheduledAction> action = "" ScheduledAction(imp->frame()->script()->currentWorldContext(), v8::Handle<v8::Function>::Cast(function), paramCount, params, args.GetIsolate()));
// FIXME: We should use OwnArrayPtr for params.
delete[] params;
@@ -131,7 +131,7 @@
if (imp->document() && !imp->document()->contentSecurityPolicy()->allowEval())
return v8Integer(0, args.GetIsolate());
ASSERT(imp->frame());
- id = DOMTimer::install(scriptContext, adoptPtr(new ScheduledAction(imp->frame()->script()->currentWorldContext(), functionString)), timeout, singleShot);
+ id = DOMTimer::install(scriptContext, adoptPtr(new ScheduledAction(imp->frame()->script()->currentWorldContext(), functionString, KURL(), args.GetIsolate())), timeout, singleShot);
}
// Try to do the idle notification before the timeout expires to get better
Modified: branches/chromium/1410/Source/WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp (142623 => 142624)
--- branches/chromium/1410/Source/WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp 2013-02-12 15:29:53 UTC (rev 142623)
+++ branches/chromium/1410/Source/WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp 2013-02-12 15:33:02 UTC (rev 142624)
@@ -70,7 +70,7 @@
return v8Integer(0, args.GetIsolate());
}
WTF::String stringFunction = toWebCoreString(function);
- timerId = DOMTimer::install(workerContext, adoptPtr(new ScheduledAction(v8Context, stringFunction, workerContext->url())), timeout, singleShot);
+ timerId = DOMTimer::install(workerContext, adoptPtr(new ScheduledAction(v8Context, stringFunction, workerContext->url(), args.GetIsolate())), timeout, singleShot);
} else if (function->IsFunction()) {
size_t paramCount = argumentCount >= 2 ? argumentCount - 2 : 0;
v8::Local<v8::Value>* params = 0;
@@ -80,7 +80,7 @@
params[i] = args[i+2];
}
// ScheduledAction takes ownership of actual params and releases them in its destructor.
- OwnPtr<ScheduledAction> action = "" ScheduledAction(v8Context, v8::Handle<v8::Function>::Cast(function), paramCount, params));
+ OwnPtr<ScheduledAction> action = "" ScheduledAction(v8Context, v8::Handle<v8::Function>::Cast(function), paramCount, params, args.GetIsolate()));
// FIXME: We should use a OwnArrayPtr for params.
delete [] params;
timerId = DOMTimer::install(workerContext, action.release(), timeout, singleShot);
