Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Oct 23, 2018

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

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}

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}
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 675cff1 into master Oct 23, 2018
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-892067a4c2 branch October 23, 2018 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants