New issue
Advanced search Search tips

Issue 702218 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Add a DCHECK for ExecutionContext::isContextSuspended to v8 bindings

Project Member Reported by altimin@chromium.org, Mar 16 2017

Issue description

In  http://crbug.com/700792  one of the root causes was that v8 bindings callbacks were cancelled silently when ExecutionContext was suspended. 

Add a DCHECK (and a test?) to make sure that we catch these errors. 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 12 2017

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

commit 7e5e14e80e181ad914e09d9f607281f248ec49e6
Author: altimin <altimin@chromium.org>
Date: Wed Apr 12 01:08:10 2017

DCHECK for execution context being unsuspended during v8 bindings callback.

To prevent loss of data, change silent failure when trying to run a v8 callback on a suspended execution context to a DCHECK.

R=haraken@chromium.org
CC=skyostil@chromium.org

BUG= 702218 , 702160 

Review-Url: https://codereview.chromium.org/2800093002
Cr-Commit-Position: refs/heads/master@{#463869}

[modify] https://crrev.com/7e5e14e80e181ad914e09d9f607281f248ec49e6/third_party/WebKit/Source/bindings/templates/callback_interface.cpp.tmpl
[modify] https://crrev.com/7e5e14e80e181ad914e09d9f607281f248ec49e6/third_party/WebKit/Source/bindings/tests/results/core/V8TestCallbackInterface.cpp

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 19 2017

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

commit a8835855fb816d7f96b3b009d52b814129a02bde
Author: hablich <hablich@chromium.org>
Date: Wed Apr 19 15:47:35 2017

Revert of DCHECK for execution context being unsuspended during v8 bindings callback. (patchset #2 id:20001 of https://codereview.chromium.org/2800093002/ )

Reason for revert:
Speculative revert because of
BUG=711196

Original issue's description:
> DCHECK for execution context being unsuspended during v8 bindings callback.
>
> To prevent loss of data, change silent failure when trying to run a v8 callback on a suspended execution context to a DCHECK.
>
> R=haraken@chromium.org
> CC=skyostil@chromium.org
>
> BUG= 702218 , 702160 
>
> Review-Url: https://codereview.chromium.org/2800093002
> Cr-Commit-Position: refs/heads/master@{#463869}
> Committed: https://chromium.googlesource.com/chromium/src/+/7e5e14e80e181ad914e09d9f607281f248ec49e6

TBR=haraken@chromium.org,altimin@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 702218 , 702160 

Review-Url: https://codereview.chromium.org/2825363002
Cr-Commit-Position: refs/heads/master@{#465622}

[modify] https://crrev.com/a8835855fb816d7f96b3b009d52b814129a02bde/third_party/WebKit/Source/bindings/templates/callback_interface.cpp.tmpl
[modify] https://crrev.com/a8835855fb816d7f96b3b009d52b814129a02bde/third_party/WebKit/Source/bindings/tests/results/core/V8TestCallbackInterface.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 26 2017

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

commit 89bf4588a7238e2da23c119ad363edab624e73e4
Author: altimin <altimin@chromium.org>
Date: Wed Apr 26 21:20:35 2017

DCHECK for execution context being unsuspended during v8 bindings callback.

To prevent loss of data, change silent failure when trying to run a v8 callback on a suspended execution context to a DCHECK.

R=haraken@chromium.org
CC=skyostil@chromium.org

BUG= 702218 , 702160 

Review-Url: https://codereview.chromium.org/2800093002
Cr-Original-Commit-Position: refs/heads/master@{#463869}
Committed: https://chromium.googlesource.com/chromium/src/+/7e5e14e80e181ad914e09d9f607281f248ec49e6
Review-Url: https://codereview.chromium.org/2800093002
Cr-Commit-Position: refs/heads/master@{#467459}

[modify] https://crrev.com/89bf4588a7238e2da23c119ad363edab624e73e4/third_party/WebKit/Source/bindings/templates/callback_interface.cpp.tmpl
[modify] https://crrev.com/89bf4588a7238e2da23c119ad363edab624e73e4/third_party/WebKit/Source/bindings/tests/results/core/V8TestCallbackInterface.cpp

Status: Fixed (was: Available)

Sign in to add a comment