New issue
Advanced search Search tips

Issue 883036 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 2
Type: Bug

Blocking:
issue 866610



Sign in to add a comment

Incumbent settings object is sometimes set incorrectly

Project Member Reported by timothygu@chromium.org, Sep 11

Issue description

When I am converting setTimeout() to use V8Function (see  bug 866610 ), https://github.com/web-platform-tests/wpt/blob/master/html/browsers/browsing-the-web/unloading-documents/unload/007.html consistently times out.

The linked test does a series of navigation in an iframe. It goes roughly like this:

1. 007.html creates a frame with 007-1.html
2. 007-1.html then navigates to 007-2.html
3. 007-2.html calls the `start_test()` function defined in 007.html, which sets a timer to call `t.done()`, thus finishing the test
4. 007-2.html navigates away from itself using `history.back()`

The issue here is that the timeout scheduled by `start_test()` is not getting called, and that is because the incumbent settings object is incorrectly set to be 007-2.html's, which by that point is already detached, rather than 007.html's.

However, this doesn't seem to be what the HTML Standard says. For this case, it seems like by definition ("the most-recently-entered author function or script on the stack, or the author function or script that originally scheduled the currently-running callback.") the incumbent realm when `setTimeout()` is called should be 007.html's, as `start_test()` is defined in 007.html.
 
Cc: peria@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 13

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

commit 726e27938707685d747b1800defffb46d8ae39e0
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Thu Sep 13 16:31:37 2018

Fix Isolate::GetIncumbentContext().

It turned out that the original implementation was broken
from the beginning. This patch fixes the API to return
the correct one.

GetIncumbentContext was implemented at
https://chromium-review.googlesource.com/c/v8/v8/+/536728

Change-Id: Iba29171bac10ed82575a8079396768a9d5af3b13
Bug:  chromium:883036 
Reviewed-on: https://chromium-review.googlesource.com/1219368
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55874}
[modify] https://crrev.com/726e27938707685d747b1800defffb46d8ae39e0/src/isolate.cc
[modify] https://crrev.com/726e27938707685d747b1800defffb46d8ae39e0/test/unittests/api/isolate-unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment