Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
721033004:
Implement WindowTimers.set{Timeout,Interval} without [Custom] (Closed)
|
Created:
6 years, 1 month ago by Jens Widell Modified:
6 years, 1 month ago CC:
arv+blink, blink-reviews, blink-reviews-bindings_chromium.org, Inactive Base URL:
https://chromium.googlesource.com/chromium/blink.git@idl-overload-with-variadic Project:
blink Visibility:
Public. |
DescriptionImplement WindowTimers.set{Timeout,Interval} without [Custom]
This allows us to drop two fairly big and largely duplicated chunks of
custom bindings code in V8WindowCustom.cpp/V8WorkerGlobalScopeCustom.cpp.
As a consequence, some more logic is added to DOMWindowTimers.cpp, which
is now responsible for doing the "is this allowed?" checks, and creating
the ScheduledAction objects.
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185538
Patch Set 1 #
Total comments: 15
Patch Set 2 : address comments #Patch Set 3 : rebased #
Total comments: 12
Patch Set 4 : style fixes #Patch Set 5 : more style fixes #Patch Set 6 : resurrect call to V8GCForContextDispose::notifyIdle() #Patch Set 7 : drop some more includes #
Total comments: 1
Patch Set 8 : refine notifyIdle hack #Patch Set 9 : rebased #
Messages
Total messages: 33 (6 generated)
jl@opera.com changed reviewers: + haraken@chromium.org
PTAL The try-bot results for PS#1 are irrelevant; it's being tested without https://codereview.chromium.org/723013003/ which it depends on for correctness.
https://codereview.chromium.org/721033004/diff/1/Source/bindings/core/v8/cust... File Source/bindings/core/v8/custom/V8WindowCustom.cpp (left): https://codereview.chromium.org/721033004/diff/1/Source/bindings/core/v8/cust... Source/bindings/core/v8/custom/V8WindowCustom.cpp:83: exceptionState.throwDOMException(InvalidAccessError, "No script context is available in which to execute the script."); This exception is no longer thrown. The new generated code includes the BindingsSecurity::shouldAllowAccessToFrame() call (further down in this code) which fails silently if impl->frame() is null. https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... File Source/core/frame/DOMWindowTimers.cpp (right): https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:81: ExecutionContext* executionContext = eventTarget.executionContext(); The old custom code in V8WindowCustom.cpp had an !impl->document() check. An equivalent check here would be to check if executionContext is null, I guess.
https://codereview.chromium.org/721033004/diff/1/Source/bindings/core/v8/Sche... File Source/bindings/core/v8/ScheduledAction.cpp (right): https://codereview.chromium.org/721033004/diff/1/Source/bindings/core/v8/Sche... Source/bindings/core/v8/ScheduledAction.cpp:49: ScheduledAction::ScheduledAction(ScriptState* scriptState, const ScriptValue& function, const Vector<ScriptValue>& arguments, v8::Isolate* isolate) You can drop the v8::Isolate* parameter, since you can retrieve it from ScriptState. https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... File Source/core/frame/DOMWindowTimers.cpp (right): https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:52: return false; Shouldn't this return true? https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:53: return true; return false? https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:61: return false; I guess this should be: if (isEval && policy && policy->allowEval()) return true; return false; ? https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:74: static PassOwnPtr<ScheduledAction> makeScheduledAction(ScriptState* scriptState, String handler) Instead of adding makeScheduledAction(), we should add SchedulerAction::create(). https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:81: ExecutionContext* executionContext = eventTarget.executionContext(); On 2014/11/13 12:37:47, Jens Widell wrote: > The old custom code in V8WindowCustom.cpp had an !impl->document() check. An > equivalent check here would be to check if executionContext is null, I guess. I think we should check against scriptState->executionContext(). scriptState->executionContext() is more reliable because it retrieves a right ExecutionContext from the current ScriptState. https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:82: if (!isAllowed(executionContext, false)) Do we need to call isAllowed() when isEval==false? If isEval==false, I guess isAllowed() should always return true.
https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... File Source/core/frame/DOMWindowTimers.cpp (right): https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:52: return false; On 2014/11/13 12:59:31, haraken wrote: > > Shouldn't this return true? I was intending to have if (!allowEval()) return false; return true; but lost the ! somewhere along the way. Since we only have one check for documents, the other way works too. If we had multiple independent checks, "if (!condition) return false; return true;" scales better. https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:81: ExecutionContext* executionContext = eventTarget.executionContext(); On 2014/11/13 12:59:31, haraken wrote: > On 2014/11/13 12:37:47, Jens Widell wrote: > > The old custom code in V8WindowCustom.cpp had an !impl->document() check. An > > equivalent check here would be to check if executionContext is null, I guess. > > I think we should check against scriptState->executionContext(). > scriptState->executionContext() is more reliable because it retrieves a right > ExecutionContext from the current ScriptState. But isn't scriptState the calling script, whereas impl->executionContext() (or eventTarget.executionContext() here) is the environment in which we should set the timeout? For "window.setTimeout(...)" they're the same, but for "iframe.contentWindow.setTimeout(...)" they're not. I think. https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:82: if (!isAllowed(executionContext, false)) On 2014/11/13 12:59:32, haraken wrote: > > Do we need to call isAllowed() when isEval==false? If isEval==false, I guess > isAllowed() should always return true. > I might be badly named for what I intended it to be. I meant to put all the checks from the old custom code that caused us to bail out early in it. For instance the "workerGlobalScope->script()" check, which is unrelated to whether it's an "eval".
https://codereview.chromium.org/721033004/diff/1/Source/bindings/core/v8/Sche... File Source/bindings/core/v8/ScheduledAction.cpp (right): https://codereview.chromium.org/721033004/diff/1/Source/bindings/core/v8/Sche... Source/bindings/core/v8/ScheduledAction.cpp:49: ScheduledAction::ScheduledAction(ScriptState* scriptState, const ScriptValue& function, const Vector<ScriptValue>& arguments, v8::Isolate* isolate) On 2014/11/13 12:59:31, haraken wrote: > > You can drop the v8::Isolate* parameter, since you can retrieve it from > ScriptState. Done. https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... File Source/core/frame/DOMWindowTimers.cpp (right): https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:52: return false; On 2014/11/13 13:12:42, Jens Widell wrote: > On 2014/11/13 12:59:31, haraken wrote: > > > > Shouldn't this return true? > > I was intending to have > > if (!allowEval()) > return false; > return true; > > but lost the ! somewhere along the way. Since we only have one check for > documents, the other way works too. If we had multiple independent checks, "if > (!condition) return false; return true;" scales better. I've fixed the logic here to be what I initially intended. https://codereview.chromium.org/721033004/diff/1/Source/core/frame/DOMWindowT... Source/core/frame/DOMWindowTimers.cpp:74: static PassOwnPtr<ScheduledAction> makeScheduledAction(ScriptState* scriptState, String handler) On 2014/11/13 12:59:32, haraken wrote: > > Instead of adding makeScheduledAction(), we should add > SchedulerAction::create(). Done.
LGTM
Status update: This patch breaks a lot of tests according to the try-bots. Unfortunately, I can't seem to reproduce the breakage locally. Need to figure out what's going on.
haraken@chromium.org changed reviewers: + yutak@chromium.org
+yutak I think setTimeout has a very good test coverage.
On 2014/11/14 07:14:24, haraken wrote: > +yutak > > I think setTimeout has a very good test coverage. Yes, breaking setTimeout() certainly breaks half the world. That's a good thing; I don't want to break it. :-)
https://codereview.chromium.org/721033004/diff/40001/Source/bindings/core/v8/... File Source/bindings/core/v8/custom/V8WindowCustom.cpp (left): https://codereview.chromium.org/721033004/diff/40001/Source/bindings/core/v8/... Source/bindings/core/v8/custom/V8WindowCustom.cpp:145: V8GCForContextDispose::instance().notifyIdle(); I've lost this call, BTW. That was not intentional.
LGTM, just a number of stylistic issues. You need to figure out what's happening in the trybots, though. Here's one of the results: https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/... Seems like you've managed to hit some assertion. You might need a Debug build to reproduce this. https://codereview.chromium.org/721033004/diff/40001/Source/bindings/core/v8/... File Source/bindings/core/v8/ScheduledAction.h (right): https://codereview.chromium.org/721033004/diff/40001/Source/bindings/core/v8/... Source/bindings/core/v8/ScheduledAction.h:50: static PassOwnPtr<ScheduledAction> create(ScriptState*, const ScriptValue&, const Vector<ScriptValue>&); The meaning of the second and third arguments is not immediately obvious; you could put parameter names for these. https://codereview.chromium.org/721033004/diff/40001/Source/bindings/core/v8/... Source/bindings/core/v8/ScheduledAction.h:51: static PassOwnPtr<ScheduledAction> create(ScriptState*, const String&); Ditto for the second argument. https://codereview.chromium.org/721033004/diff/40001/Source/bindings/core/v8/... Source/bindings/core/v8/ScheduledAction.h:59: ScheduledAction(ScriptState*, const String&); Ditto. https://codereview.chromium.org/721033004/diff/40001/Source/core/frame/DOMWin... File Source/core/frame/DOMWindowTimers.cpp (right): https://codereview.chromium.org/721033004/diff/40001/Source/core/frame/DOMWin... Source/core/frame/DOMWindowTimers.cpp:83: // (Bug 1009597) I know that's not something you wrote, but I had initially no idea about where the number came from ;) This actually came from Google's internal bug tracker, and the bug was essentially about some sites taking 40-50% CPU for empty handlers. [BTW, this should not be the case with our current implementation, because we now align the tasks in 4ms granurality, IIRC. However, there's no strong reason to drop this early return, so I think we can just keep this optimization.] So, to summarize, you can simply drop the bug number, and possibly add some comments mentioning that some empty hander was a CPU hog in the past. https://codereview.chromium.org/721033004/diff/40001/Source/core/frame/DOMWin... Source/core/frame/DOMWindowTimers.cpp:105: // (Bug 1009597) Ditto. https://codereview.chromium.org/721033004/diff/40001/Source/core/frame/DOMWin... File Source/core/frame/DOMWindowTimers.h (right): https://codereview.chromium.org/721033004/diff/40001/Source/core/frame/DOMWin... Source/core/frame/DOMWindowTimers.h:47: int setTimeout(ScriptState*, EventTarget&, String, int timeout, const Vector<ScriptValue>&); String -> const String& ? Also some parameter names are desirable for documentation purposes.
On 2014/11/14 08:24:06, Yuta Kitamura wrote: > LGTM, just a number of stylistic issues. Thanks! Issues addressed in PS#4 and PS#5. > You need to figure out what's happening in the trybots, > though. Here's one of the results: > https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/... > > Seems like you've managed to hit some assertion. You might > need a Debug build to reproduce this. Thanks for the investigation and link! Compiling with dcheck_always_on=1 made the issue appear locally. Issue fixed by https://codereview.chromium.org/732483002/. https://codereview.chromium.org/721033004/diff/40001/Source/bindings/core/v8/... File Source/bindings/core/v8/ScheduledAction.h (right): https://codereview.chromium.org/721033004/diff/40001/Source/bindings/core/v8/... Source/bindings/core/v8/ScheduledAction.h:50: static PassOwnPtr<ScheduledAction> create(ScriptState*, const ScriptValue&, const Vector<ScriptValue>&); On 2014/11/14 08:24:06, Yuta Kitamura wrote: > The meaning of the second and third arguments is not immediately > obvious; you could put parameter names for these. Done. https://codereview.chromium.org/721033004/diff/40001/Source/bindings/core/v8/... Source/bindings/core/v8/ScheduledAction.h:51: static PassOwnPtr<ScheduledAction> create(ScriptState*, const String&); On 2014/11/14 08:24:06, Yuta Kitamura wrote: > Ditto for the second argument. Done. https://codereview.chromium.org/721033004/diff/40001/Source/bindings/core/v8/... Source/bindings/core/v8/ScheduledAction.h:59: ScheduledAction(ScriptState*, const String&); On 2014/11/14 08:24:06, Yuta Kitamura wrote: > Ditto. Done. https://codereview.chromium.org/721033004/diff/40001/Source/core/frame/DOMWin... File Source/core/frame/DOMWindowTimers.cpp (right): https://codereview.chromium.org/721033004/diff/40001/Source/core/frame/DOMWin... Source/core/frame/DOMWindowTimers.cpp:83: // (Bug 1009597) On 2014/11/14 08:24:06, Yuta Kitamura wrote: > I know that's not something you wrote, but I had initially no > idea about where the number came from ;) > > This actually came from Google's internal bug tracker, and > the bug was essentially about some sites taking 40-50% CPU for > empty handlers. > > [BTW, this should not be the case with our current implementation, > because we now align the tasks in 4ms granurality, IIRC. However, > there's no strong reason to drop this early return, so I think we > can just keep this optimization.] > > So, to summarize, you can simply drop the bug number, and possibly > add some comments mentioning that some empty hander was a CPU hog > in the past. Thanks for the information! I did wonder what kind of bug number it was, but since I just copied the code, I didn't wonder enough to take any action. :-) Comment(s) modified slightly. https://codereview.chromium.org/721033004/diff/40001/Source/core/frame/DOMWin... File Source/core/frame/DOMWindowTimers.h (right): https://codereview.chromium.org/721033004/diff/40001/Source/core/frame/DOMWin... Source/core/frame/DOMWindowTimers.h:47: int setTimeout(ScriptState*, EventTarget&, String, int timeout, const Vector<ScriptValue>&); On 2014/11/14 08:24:06, Yuta Kitamura wrote: > String -> const String& ? > > Also some parameter names are desirable for documentation > purposes. Done.
haraken@: PTAL? In PS#6 I've resurrected the call to V8GCForContextDispose::notifyIdle() that was in the old custom code. I don't really know what it does. Notable difference: we now call it for both the Window and WorkerGlobalScope cases, whereas previously only V8WindowCustom.cpp had the call. Is this good or bad? https://codereview.chromium.org/721033004/diff/120001/Source/core/frame/DOMWi... File Source/core/frame/DOMWindowTimers.cpp (right): https://codereview.chromium.org/721033004/diff/120001/Source/core/frame/DOMWi... Source/core/frame/DOMWindowTimers.cpp:69: static void notifyIdle() This is a helper function to avoid duplication, but more of the comment than the code, really.
> Notable difference: we now call it for both the Window and WorkerGlobalScope cases, whereas previously only V8WindowCustom.cpp had the call. Is this good or bad? Hmm, adding extra idl notifications is doing something wrong. Ideally we should just remove the idle notification completely, but I'm not sure if it is OK to remove the existing idle notification from window's setTimeout. I'd like to defer the review for jochen@.
haraken@chromium.org changed reviewers: + jochen@chromium.org
+jochen
this idle call is really only used for sunspider. we should trigger it as little as possible, so adding it to worker global scope does not make sense.
On 2014/11/17 10:34:03, jochen (slow) wrote: > this idle call is really only used for sunspider. we should trigger it as little > as possible, so adding it to worker global scope does not make sense. But keeping it for the Window case is necessary?
On 2014/11/17 at 10:35:39, jl wrote: > On 2014/11/17 10:34:03, jochen (slow) wrote: > > this idle call is really only used for sunspider. we should trigger it as little > > as possible, so adding it to worker global scope does not make sense. > > But keeping it for the Window case is necessary? yes. sunspider has a setTimeout() call after navigating its iframe to the next test, but before running the test. In the standalone version, there's an explicit gc() call, but if you run it in the browser, there's not. I guess the idea was that the JS engine would somehow automatically do GC if you just give it some time, or something. Of course, this unconditional gc after iframe navigation would totally ruin performance on any other web site, so there's a series of other conditions that have to be hit for this idle notification to actually trigger a gc. tl;dr: sadness.
On 2014/11/17 10:39:15, jochen (slow) wrote: > On 2014/11/17 at 10:35:39, jl wrote: > > On 2014/11/17 10:34:03, jochen (slow) wrote: > > > this idle call is really only used for sunspider. we should trigger it as > little > > > as possible, so adding it to worker global scope does not make sense. > > > > But keeping it for the Window case is necessary? > > yes. > > sunspider has a setTimeout() call after navigating its iframe to the next test, > but before running the test. Should we limit this to setTimeout()? With this CL we'd need to do it explicitly for both setTimeout() and setInterval(), so we can easily avoid it for setInterval(). (Not that it was hard in the old code, of course.) > tl;dr: sadness. Sunspider was a test that inspired all sorts of hacks, I think. I'd be lying if I said we didn't do various rather peculiar things in Presto as well. :-)
setTimeout only would make sense, yes
On 2014/11/17 10:51:25, jochen (slow) wrote: > setTimeout only would make sense, yes Patch updated to only call notifyIdle() from setTimeout(), and only do it in the document (non-worker) case.
On 2014/11/18 09:34:28, Jens Widell wrote: > On 2014/11/17 10:51:25, jochen (slow) wrote: > > setTimeout only would make sense, yes > > Patch updated to only call notifyIdle() from setTimeout(), and only do it in the > document (non-worker) case. LGTM
lgtm
The CQ bit was checked by jl@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/721033004/140001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for Source/bindings/core/v8/custom/V8WindowCustom.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file Source/bindings/core/v8/custom/V8WindowCustom.cpp
Hunk #2 FAILED at 65.
1 out of 3 hunks FAILED -- saving rejects to file
Source/bindings/core/v8/custom/V8WindowCustom.cpp.rej
Patch: Source/bindings/core/v8/custom/V8WindowCustom.cpp
Index: Source/bindings/core/v8/custom/V8WindowCustom.cpp
diff --git a/Source/bindings/core/v8/custom/V8WindowCustom.cpp
b/Source/bindings/core/v8/custom/V8WindowCustom.cpp
index
4c613dd51c7b494bebfb027ab4aa1a05f3f91dc0..aa4ec1f3989bbefe2790acf8f515b00bc3d4bd0b
100644
--- a/Source/bindings/core/v8/custom/V8WindowCustom.cpp
+++ b/Source/bindings/core/v8/custom/V8WindowCustom.cpp
@@ -34,22 +34,18 @@
#include "bindings/core/v8/BindingSecurity.h"
#include "bindings/core/v8/ExceptionMessages.h"
#include "bindings/core/v8/ExceptionState.h"
-#include "bindings/core/v8/ScheduledAction.h"
#include "bindings/core/v8/ScriptController.h"
#include "bindings/core/v8/ScriptSourceCode.h"
#include "bindings/core/v8/SerializedScriptValue.h"
#include "bindings/core/v8/V8Binding.h"
#include "bindings/core/v8/V8EventListener.h"
#include "bindings/core/v8/V8EventListenerList.h"
-#include "bindings/core/v8/V8GCForContextDispose.h"
#include "bindings/core/v8/V8HTMLCollection.h"
#include "bindings/core/v8/V8HiddenValue.h"
#include "bindings/core/v8/V8Node.h"
#include "core/dom/DOMArrayBuffer.h"
#include "core/dom/ExceptionCode.h"
#include "core/dom/MessagePort.h"
-#include "core/frame/DOMTimer.h"
-#include "core/frame/DOMWindowTimers.h"
#include "core/frame/FrameView.h"
#include "core/frame/LocalDOMWindow.h"
#include "core/frame/LocalFrame.h"
@@ -69,84 +65,6 @@
namespace blink {
-// FIXME: There is a lot of duplication with SetTimeoutOrInterval() in
V8WorkerGlobalScopeCustom.cpp.
-// We should refactor this.
-static void windowSetTimeoutImpl(const v8::FunctionCallbackInfo<v8::Value>&
info, bool singleShot, ExceptionState& exceptionState)
-{
- int argumentCount = info.Length();
-
- if (argumentCount < 1)
- return;
-
- LocalDOMWindow* impl = toLocalDOMWindow(V8Window::toImpl(info.Holder()));
- if (!impl->frame() || !impl->document()) {
- exceptionState.throwDOMException(InvalidAccessError, "No script context
is available in which to execute the script.");
- return;
- }
- ScriptState* scriptState = ScriptState::current(info.GetIsolate());
- v8::Handle<v8::Value> function = info[0];
- String functionString;
- if (!function->IsFunction()) {
- if (function->IsString()) {
- functionString = toCoreString(function.As<v8::String>());
- } else {
- v8::Handle<v8::String> v8String = function->ToString();
-
- // Bail out if string conversion failed.
- if (v8String.IsEmpty())
- return;
-
- functionString = toCoreString(v8String);
- }
-
- // Don't allow setting timeouts to run empty functions!
- // (Bug 1009597)
- if (!functionString.length())
- return;
- }
-
- if (!BindingSecurity::shouldAllowAccessToFrame(info.GetIsolate(),
impl->frame(), exceptionState))
- return;
-
- OwnPtr<ScheduledAction> action;
- if (function->IsFunction()) {
- int paramCount = argumentCount >= 2 ? argumentCount - 2 : 0;
- OwnPtr<v8::Local<v8::Value>[]> params;
- if (paramCount > 0) {
- params = adoptArrayPtr(new v8::Local<v8::Value>[paramCount]);
- for (int i = 0; i < paramCount; i++) {
- // parameters must be globalized
- params[i] = info[i+2];
- }
- }
-
- // params is passed to action, and released in action's destructor
- ASSERT(impl->frame());
- action = adoptPtr(new ScheduledAction(scriptState,
v8::Handle<v8::Function>::Cast(function), paramCount, params.get(),
info.GetIsolate()));
- } else {
- if (impl->document() &&
!impl->document()->contentSecurityPolicy()->allowEval()) {
- v8SetReturnValue(info, 0);
- return;
- }
- ASSERT(impl->frame());
- action = adoptPtr(new ScheduledAction(scriptState, functionString,
KURL(), info.GetIsolate()));
- }
-
- int32_t timeout = argumentCount >= 2 ? info[1]->Int32Value() : 0;
- int timerId;
- if (singleShot)
- timerId = DOMWindowTimers::setTimeout(*impl, action.release(),
timeout);
- else
- timerId = DOMWindowTimers::setInterval(*impl, action.release(),
timeout);
-
- // FIXME: Crude hack that attempts to pass idle time to V8. This should be
- // done using the scheduler instead.
- if (timeout >= 0)
- V8GCForContextDispose::instance().notifyIdle();
-
- v8SetReturnValue(info, timerId);
-}
-
void V8Window::eventAttributeGetterCustom(const
v8::PropertyCallbackInfo<v8::Value>& info)
{
LocalFrame* frame =
toLocalDOMWindow(V8Window::toImpl(info.Holder()))->frame();
@@ -438,22 +356,6 @@ void
V8Window::namedPropertyGetterCustom(v8::Local<v8::String> name, const v8::P
}
}
-
-void V8Window::setTimeoutMethodCustom(const
v8::FunctionCallbackInfo<v8::Value>& info)
-{
- ExceptionState exceptionState(ExceptionState::ExecutionContext,
"setTimeout", "Window", info.Holder(), info.GetIsolate());
- windowSetTimeoutImpl(info, true, exceptionState);
- exceptionState.throwIfNeeded();
-}
-
-
-void V8Window::setIntervalMethodCustom(const
v8::FunctionCallbackInfo<v8::Value>& info)
-{
- ExceptionState exceptionState(ExceptionState::ExecutionContext,
"setInterval", "Window", info.Holder(), info.GetIsolate());
- windowSetTimeoutImpl(info, false, exceptionState);
- exceptionState.throwIfNeeded();
-}
-
bool V8Window::namedSecurityCheckCustom(v8::Local<v8::Object> host,
v8::Local<v8::Value> key, v8::AccessType type, v8::Local<v8::Value>)
{
v8::Isolate* isolate = v8::Isolate::GetCurrent();
The CQ bit was checked by jl@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/721033004/160001
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as https://src.chromium.org/viewvc/blink?view=rev&revision=185538 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
