- Revision
- 142565
- Author
- [email protected]
- Date
- 2013-02-11 18:06:13 -0800 (Mon, 11 Feb 2013)
Log Message
[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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (142564 => 142565)
--- trunk/Source/WebCore/ChangeLog 2013-02-12 01:58:38 UTC (rev 142564)
+++ trunk/Source/WebCore/ChangeLog 2013-02-12 02:06:13 UTC (rev 142565)
@@ -1,3 +1,35 @@
+2013-02-11 Kentaro Hara <[email protected]>
+
+ [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):
+
2013-02-11 Adenilson Cavalcanti <[email protected]>
Build fix: r142549 broke EFL build
Modified: trunk/Source/WebCore/bindings/v8/ScheduledAction.cpp (142564 => 142565)
--- trunk/Source/WebCore/bindings/v8/ScheduledAction.cpp 2013-02-12 01:58:38 UTC (rev 142564)
+++ trunk/Source/WebCore/bindings/v8/ScheduledAction.cpp 2013-02-12 02:06:13 UTC (rev 142565)
@@ -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: trunk/Source/WebCore/bindings/v8/ScheduledAction.h (142564 => 142565)
--- trunk/Source/WebCore/bindings/v8/ScheduledAction.h 2013-02-12 01:58:38 UTC (rev 142564)
+++ trunk/Source/WebCore/bindings/v8/ScheduledAction.h 2013-02-12 02:06:13 UTC (rev 142565)
@@ -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: trunk/Source/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp (142564 => 142565)
--- trunk/Source/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp 2013-02-12 01:58:38 UTC (rev 142564)
+++ trunk/Source/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp 2013-02-12 02:06:13 UTC (rev 142565)
@@ -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: trunk/Source/WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp (142564 => 142565)
--- trunk/Source/WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp 2013-02-12 01:58:38 UTC (rev 142564)
+++ trunk/Source/WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp 2013-02-12 02:06:13 UTC (rev 142565)
@@ -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);
