New issue
Advanced search Search tips

Issue 866610 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 28
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocked on:
issue 883036
issue 888025
issue 888867



Sign in to add a comment

ScheduledAction does not explicitly enter the callback's context

Project Member Reported by timothygu@chromium.org, Jul 23

Issue description

Chrome Version: 70.0.3500.0 (Official Build) canary (64-bit)
OS: macOS 10.13.6

What steps will reproduce the problem?

Serve the provided timer-entry.html and timer-entry.frame.html with a static HTTP server. Launch timer-entry.html.

What is the expected result?

http://localhost:8080/timer-entry.html

This is also the result on Firefox and Safari.

What happens instead?

http://localhost:8080/timer-entry.frame.html

----

Per step 23 of document open steps [1], the URL of the document of the entry settings object should be propagated to the frame's document when document.open() is called. In this case, the entry settings object should be the settings object associated with timer-entry.html. The document.open() function is called from the timer callback, which is in turn called through the "invoke a callback function" algorithm [2] by the timer initialization steps in the HTML Standard [3] (step 7.2, first case). The "invoke a callback function" algorithm then "prepares to run script" [4], which is the algorithm responsible for setting up the correct entry settings object.

Indeed, the source for document.open() (Document::open()) does correctly use the entered document's URL [5]. Yet, the ScheduledAction::Execute() function [6], responsible for invoking the timer callback, calls the function directly instead of going through the IDL Invoke() system, which correctly enters the callback's context [7].

[1]: https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#document-open-steps
[2]: https://heycam.github.io/webidl/#invoke-a-callback-function
[3]: https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timer-initialisation-steps
[4]: https://html.spec.whatwg.org/multipage/webappapis.html#prepare-to-run-script
[5]: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/document.cc?l=3025-3027&rcl=957f866abf50ff69a919bc2db001e326e142cf38
[6]: https://cs.chromium.org/chromium/src/third_party/blink/renderer/bindings/core/v8/scheduled_action.cc?l=167-169&rcl=653b14f24eee18f1665d8055e86c15811d6166da
[7]: https://cs.chromium.org/chromium/src/out/Debug/gen/third_party/blink/renderer/bindings/core/v8/v8_void_function.cc?dr=CSs&g=0&l=62-64
 
timer-entry.html
310 bytes View Download
timer-entry.frame.html
50 bytes View Download
Cc: peria@chromium.org
Owner: yukishiino@chromium.org
Status: Assigned (was: Untriaged)
Owner: timothygu@chromium.org
Status: Started (was: Assigned)
Components: Blink>Bindings
Blockedon: 883036
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/254369a5f6df06c2c6be067d14c2cb2a036ba173

commit 254369a5f6df06c2c6be067d14c2cb2a036ba173
Author: Timothy Gu <timothygu@chromium.org>
Date: Fri Sep 21 07:49:21 2018

bindings: Implement timers with V8Function

This fixes  bug 866610  by using the IDL infrastructure to properly enter
the v8::Context before calling the registered callback.

Also ensure eager finalization of ScheduledAction in DOMTimer to
prevent a memory leak. Added two more effective DCHECKs to confirm.

Bug:  866610 
Change-Id: I37d7bd05f035fe31856cfe68bae51aa0632cd3b1
Reviewed-on: https://chromium-review.googlesource.com/1220486
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Timothy Gu <timothygu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593108}
[add] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/resources/url-entry-document-timer-frame.html
[add] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url-entry-document.window.js
[modify] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/WebKit/LayoutTests/external/wpt/lint.whitelist
[modify] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/blink/renderer/bindings/core/v8/scheduled_action.cc
[modify] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/blink/renderer/bindings/core/v8/scheduled_action.h
[modify] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/blink/renderer/core/frame/dom_timer.cc
[modify] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/blink/renderer/core/frame/dom_timer.h
[modify] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/blink/renderer/core/frame/dom_timer_coordinator.h
[modify] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/blink/renderer/core/frame/dom_window_timers.cc
[modify] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/blink/renderer/core/frame/dom_window_timers.h
[modify] https://crrev.com/254369a5f6df06c2c6be067d14c2cb2a036ba173/third_party/blink/renderer/core/frame/window_timers.idl

Labels: M-71
Status: Fixed (was: Started)
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec21174201dc34cdd96a6b43199de8c8ddbed0f2

commit ec21174201dc34cdd96a6b43199de8c8ddbed0f2
Author: Frank Liberato <liberato@chromium.org>
Date: Fri Sep 21 18:16:40 2018

Revert "bindings: Implement timers with V8Function"

This reverts commit 254369a5f6df06c2c6be067d14c2cb2a036ba173.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> bindings: Implement timers with V8Function
> 
> This fixes  bug 866610  by using the IDL infrastructure to properly enter
> the v8::Context before calling the registered callback.
> 
> Also ensure eager finalization of ScheduledAction in DOMTimer to
> prevent a memory leak. Added two more effective DCHECKs to confirm.
> 
> Bug:  866610 
> Change-Id: I37d7bd05f035fe31856cfe68bae51aa0632cd3b1
> Reviewed-on: https://chromium-review.googlesource.com/1220486
> Reviewed-by: Nate Chapin <japhet@chromium.org>
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
> Commit-Queue: Timothy Gu <timothygu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#593108}

TBR=peria@chromium.org,yukishiino@chromium.org,haraken@chromium.org,japhet@chromium.org,timothygu@chromium.org

Change-Id: Ifaccc3374466b851fc28b10c63ed1397bdae635e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  866610 
Reviewed-on: https://chromium-review.googlesource.com/1239216
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593260}
[delete] https://crrev.com/12cdca8fbdb65814a8d624c12af24a3d35061856/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/resources/url-entry-document-timer-frame.html
[delete] https://crrev.com/12cdca8fbdb65814a8d624c12af24a3d35061856/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url-entry-document.window.js
[modify] https://crrev.com/ec21174201dc34cdd96a6b43199de8c8ddbed0f2/third_party/WebKit/LayoutTests/external/wpt/lint.whitelist
[modify] https://crrev.com/ec21174201dc34cdd96a6b43199de8c8ddbed0f2/third_party/blink/renderer/bindings/core/v8/scheduled_action.cc
[modify] https://crrev.com/ec21174201dc34cdd96a6b43199de8c8ddbed0f2/third_party/blink/renderer/bindings/core/v8/scheduled_action.h
[modify] https://crrev.com/ec21174201dc34cdd96a6b43199de8c8ddbed0f2/third_party/blink/renderer/core/frame/dom_timer.cc
[modify] https://crrev.com/ec21174201dc34cdd96a6b43199de8c8ddbed0f2/third_party/blink/renderer/core/frame/dom_timer.h
[modify] https://crrev.com/ec21174201dc34cdd96a6b43199de8c8ddbed0f2/third_party/blink/renderer/core/frame/dom_timer_coordinator.h
[modify] https://crrev.com/ec21174201dc34cdd96a6b43199de8c8ddbed0f2/third_party/blink/renderer/core/frame/dom_window_timers.cc
[modify] https://crrev.com/ec21174201dc34cdd96a6b43199de8c8ddbed0f2/third_party/blink/renderer/core/frame/dom_window_timers.h
[modify] https://crrev.com/ec21174201dc34cdd96a6b43199de8c8ddbed0f2/third_party/blink/renderer/core/frame/window_timers.idl

Status: Assigned (was: Fixed)
CL was reverted by https://chromium-review.googlesource.com/c/chromium/src/+/1239216.
Blockedon: 888025
ASAN reversions are usually due to setTimeouts being too short or long, FWIW.
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395

commit 17f4e7d2ec13d6112dfcd8aa253f340bdb00d395
Author: Timothy Gu <timothygu@chromium.org>
Date: Tue Oct 23 02:29:14 2018

Reland "bindings: Implement timers with V8Function"

This is a reland of 254369a5f6df06c2c6be067d14c2cb2a036ba173. It addresses bug
888025 by adding ASAN test expectations, as the relevant V8 feature does not
yet support running on ASAN builds.

Original change's description:
> bindings: Implement timers with V8Function
>
> This fixes  bug 866610  by using the IDL infrastructure to properly enter
> the v8::Context before calling the registered callback.
>
> Also ensure eager finalization of ScheduledAction in DOMTimer to
> prevent a memory leak. Added two more effective DCHECKs to confirm.
>
> Bug:  866610 
> Change-Id: I37d7bd05f035fe31856cfe68bae51aa0632cd3b1
> Reviewed-on: https://chromium-review.googlesource.com/1220486
> Reviewed-by: Nate Chapin <japhet@chromium.org>
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
> Commit-Queue: Timothy Gu <timothygu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#593108}

TBR=japhet@chromium.org

Bug:  866610 ,  888025 
Change-Id: Iee5c1d6917ad7770383e06a425f96000835a663a
Reviewed-on: https://chromium-review.googlesource.com/c/1239624
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Timothy Gu <timothygu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601830}
[modify] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/WebKit/LayoutTests/ASANExpectations
[add] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/resources/url-entry-document-timer-frame.html
[add] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url-entry-document.window.js
[modify] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/WebKit/LayoutTests/external/wpt/lint.whitelist
[modify] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/blink/renderer/bindings/core/v8/scheduled_action.cc
[modify] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/blink/renderer/bindings/core/v8/scheduled_action.h
[modify] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/blink/renderer/core/frame/dom_timer.cc
[modify] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/blink/renderer/core/frame/dom_timer.h
[modify] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/blink/renderer/core/frame/dom_timer_coordinator.h
[modify] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/blink/renderer/core/frame/dom_window_timers.cc
[modify] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/blink/renderer/core/frame/dom_window_timers.h
[modify] https://crrev.com/17f4e7d2ec13d6112dfcd8aa253f340bdb00d395/third_party/blink/renderer/core/frame/window_timers.idl

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/892067a4c2be953f9f6cac55e7b8f983c2203a92

commit 892067a4c2be953f9f6cac55e7b8f983c2203a92
Author: Timothy Gu <timothygu@chromium.org>
Date: Tue Oct 23 06:46:54 2018

Revert "Reland "bindings: Implement timers with V8Function""

This reverts commit 17f4e7d2ec13d6112dfcd8aa253f340bdb00d395.

Reason for revert: Still breaks ASAN

Original change's description:
> Reland "bindings: Implement timers with V8Function"
> 
> This is a reland of 254369a5f6df06c2c6be067d14c2cb2a036ba173. It addresses bug
> 888025 by adding ASAN test expectations, as the relevant V8 feature does not
> yet support running on ASAN builds.
> 
> Original change's description:
> > bindings: Implement timers with V8Function
> >
> > This fixes  bug 866610  by using the IDL infrastructure to properly enter
> > the v8::Context before calling the registered callback.
> >
> > Also ensure eager finalization of ScheduledAction in DOMTimer to
> > prevent a memory leak. Added two more effective DCHECKs to confirm.
> >
> > Bug:  866610 
> > Change-Id: I37d7bd05f035fe31856cfe68bae51aa0632cd3b1
> > Reviewed-on: https://chromium-review.googlesource.com/1220486
> > Reviewed-by: Nate Chapin <japhet@chromium.org>
> > Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> > Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
> > Commit-Queue: Timothy Gu <timothygu@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#593108}
> 
> TBR=japhet@chromium.org
> 
> Bug:  866610 ,  888025 
> Change-Id: Iee5c1d6917ad7770383e06a425f96000835a663a
> Reviewed-on: https://chromium-review.googlesource.com/c/1239624
> Reviewed-by: Nate Chapin <japhet@chromium.org>
> Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Commit-Queue: Timothy Gu <timothygu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#601830}

TBR=peria@chromium.org,yukishiino@chromium.org,haraken@chromium.org,japhet@chromium.org,timothygu@chromium.org

Change-Id: Ie4f45dfcc1adcc2ac3469eab99dba813723288f4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  866610 ,  888025 
Reviewed-on: https://chromium-review.googlesource.com/c/1296057
Commit-Queue: Timothy Gu <timothygu@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601864}
[modify] https://crrev.com/892067a4c2be953f9f6cac55e7b8f983c2203a92/third_party/WebKit/LayoutTests/ASANExpectations
[delete] https://crrev.com/26111c2602fc3e1a85d1364d1140f99594d54fce/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/resources/url-entry-document-timer-frame.html
[delete] https://crrev.com/26111c2602fc3e1a85d1364d1140f99594d54fce/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url-entry-document.window.js
[modify] https://crrev.com/892067a4c2be953f9f6cac55e7b8f983c2203a92/third_party/WebKit/LayoutTests/external/wpt/lint.whitelist
[modify] https://crrev.com/892067a4c2be953f9f6cac55e7b8f983c2203a92/third_party/blink/renderer/bindings/core/v8/scheduled_action.cc
[modify] https://crrev.com/892067a4c2be953f9f6cac55e7b8f983c2203a92/third_party/blink/renderer/bindings/core/v8/scheduled_action.h
[modify] https://crrev.com/892067a4c2be953f9f6cac55e7b8f983c2203a92/third_party/blink/renderer/core/frame/dom_timer.cc
[modify] https://crrev.com/892067a4c2be953f9f6cac55e7b8f983c2203a92/third_party/blink/renderer/core/frame/dom_timer.h
[modify] https://crrev.com/892067a4c2be953f9f6cac55e7b8f983c2203a92/third_party/blink/renderer/core/frame/dom_timer_coordinator.h
[modify] https://crrev.com/892067a4c2be953f9f6cac55e7b8f983c2203a92/third_party/blink/renderer/core/frame/dom_window_timers.cc
[modify] https://crrev.com/892067a4c2be953f9f6cac55e7b8f983c2203a92/third_party/blink/renderer/core/frame/dom_window_timers.h
[modify] https://crrev.com/892067a4c2be953f9f6cac55e7b8f983c2203a92/third_party/blink/renderer/core/frame/window_timers.idl

An patch is available at https://chromium-review.googlesource.com/c/chromium/src/+/1220486. However, it doesn't work with ASAN due to  issue 888025 , and I don't think I'm the best person to try to fix that; see https://chromium-review.googlesource.com/c/chromium/src/+/1296057#message-4ebbc1dd584640a9be39c6ff1c09b97b20916aef.

Assigning to yukishiino@ as the owner of  issue 888025 .
Blockedon: 888867
Apologies, when I referred to  issue 888025  I really meant  issue 888867 .
Owner: yukishiino@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 16 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/f379bb117f8ca0cd5903ecdc7f89fc3b6d513d68

commit f379bb117f8ca0cd5903ecdc7f89fc3b6d513d68
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Wed Nov 21 05:40:08 2018

Make Isolate::GetIncumbentContext() work fine with ASAN

When ASAN is enabled, the previous implementation of
Isolate::GetIncumbentContext didn't work well due to mixture of fake
and real stack frames.

This patch converts an address in the fake stack frame to an address
in the real stack frame so that we can compare two addresses.

Bug:  chromium:888867 ,  chromium:866610 
Change-Id: Iccf570b8555f2fbdc737b12894a2784ffdb31602
Reviewed-on: https://chromium-review.googlesource.com/c/1343709
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57662}
[modify] https://crrev.com/f379bb117f8ca0cd5903ecdc7f89fc3b6d513d68/src/isolate.cc
[modify] https://crrev.com/f379bb117f8ca0cd5903ecdc7f89fc3b6d513d68/test/unittests/api/isolate-unittest.cc

Owner: timothygu@chromium.org
Hi Timothy,

My fix in V8 at #c16 successfully rolled into Chromium, and now we can use v8::Isolate::GetIncumbentContext with ASAN enabled.  I think that now you can land your patch without a concern about ASAN.  Let me know if you still have any trouble (with or without ASAN).

I found another issue that the current implementation of v8::Isolate::GetIncumbentContext doesn't work very well with MSAN (or maybe when simulator is enabled?).  I think that I can fix it in a week or two.  Please stay tuned.

Project Member

Comment 19 by bugdroid1@chromium.org, Dec 7

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/134c67ef8abb2374488c71240347de58dadfe9e0

commit 134c67ef8abb2374488c71240347de58dadfe9e0
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Fri Dec 07 14:52:25 2018

Make Isolate::GetIncumbentContext() work well with MSan

https://crrev.com/c/1343709 fixed GetIncumbentContext to work
with ASan, however, GetIncumbentContext didn't work well with
MSan because MSan uses a simulator which supports yet another
separate stack frame.

This patch fixes GetIncumbentContext so that it works well
with not only ASan but also MSan simply following the same way
as v8::TryCatch does.

i::GetCurrentStackPosition() solves the issue of ASan and
SafeStack (native but separate stack frame), and
i::SimulatorStack solves the issue of MSan (simulator stack
frame).

Bug:  chromium:888867 ,  chromium:866610 
Change-Id: Id803cbfd17fb1b1d9b8ee34c4802768f3a2f8e79
Reviewed-on: https://chromium-review.googlesource.com/c/1356691
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58096}
[modify] https://crrev.com/134c67ef8abb2374488c71240347de58dadfe9e0/include/v8.h
[modify] https://crrev.com/134c67ef8abb2374488c71240347de58dadfe9e0/src/api.cc
[modify] https://crrev.com/134c67ef8abb2374488c71240347de58dadfe9e0/src/isolate.cc
[modify] https://crrev.com/134c67ef8abb2374488c71240347de58dadfe9e0/src/simulator.h

Hi Timothy,

I've finally fixed v8::Isolate::GetIncumbentContext.  It should be now working well with ASan, MSan, etc.  I'm sorry for having you wait so long.  Would you mind to land your patch again?

Feel free to assign the issue to me if you're busy and unlikely to work on this again.

Labels: -M-71
Owner: yukishiino@chromium.org
Hi Yuki, yeah it's exceedingly difficult for me to work on Chromium these days. Could you take over?
Sure!  Let me work on this.
Project Member

Comment 23 by bugdroid1@chromium.org, Dec 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f95130edf1c94febbf0eac48f1808ad12edf75de

commit f95130edf1c94febbf0eac48f1808ad12edf75de
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Fri Dec 28 06:31:34 2018

v8binding: Implement timers with IDL callback function of type Function

This is mostly a reland of Iee5c1d6917ad7770383e06a425f96000835a663a.

This patch implements setTimeout/setInterval Web APIs with IDL
callback function of type Function (i.e. V8Function in Blink).

Also ensure eager finalization of ScheduledAction in DOMTimer to
prevent a memory leak.

Bug:  866610 
Change-Id: I3f460247f27069e4054a984efd3f98a2ce0ceac7
Reviewed-on: https://chromium-review.googlesource.com/c/1391248
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619125}
[modify] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/renderer/bindings/core/v8/scheduled_action.cc
[modify] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/renderer/bindings/core/v8/scheduled_action.h
[modify] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/renderer/core/frame/dom_timer.cc
[modify] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/renderer/core/frame/dom_timer.h
[modify] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/renderer/core/frame/dom_timer_coordinator.h
[modify] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/renderer/core/frame/window_or_worker_global_scope.cc
[modify] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/renderer/core/frame/window_or_worker_global_scope.h
[modify] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/renderer/core/frame/window_or_worker_global_scope.idl
[add] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/web_tests/external/wpt/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/resources/url-entry-document-timer-frame.html
[add] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/web_tests/external/wpt/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url-entry-document.window.js
[modify] https://crrev.com/f95130edf1c94febbf0eac48f1808ad12edf75de/third_party/blink/web_tests/external/wpt/lint.whitelist

Status: Fixed (was: Started)

Sign in to add a comment