New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 554834 link

Starred by 13 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: 2017-03-01
OS: All
Pri: 2
Type: Bug

Blocking:
issue 154321
issue 594049


Participants' hotlists:
Lifecycle


Sign in to add a comment

visibilitychange event does not fire when unloading the document

Project Member Reported by igrigo...@chromium.org, Nov 12 2015

Issue description

Version: 46.0.2490.80
OS: all

What steps will reproduce the problem?
1. Subscribe to `visibilitychange` event
2. Force an unload of document (e.g. navigate away)
3. visibilitychange event should fire as part of unload process and document.visibilityState should report `hidden`

What is the expected output? What do you see instead?

Per spec [1], we should fire a visibilitychange event. Currently, we do not. See discussion in [2], and test page [3].

[1] http://w3c.github.io/page-visibility/#reacting-to-visibility-changes
[2] https://github.com/w3c/page-visibility/issues/18
[3] http://output.jsbin.com/zubiyid/latest/quiet?beaconUrl=...
 

Comment 1 by kinuko@chromium.org, Nov 13 2015

Owner: kinuko@chromium.org
Status: Assigned

Comment 2 by kinuko@chromium.org, Mar 11 2016

Blocking: -594049
Cc: esprehn@chromium.org dcheng@chromium.org
Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=594049

Plan: land the event change itself behind the flag to start with (aiming for M51) while we also continue adding  necessary restrictions (e.g. for nested message loop)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 12 2016

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

commit 8909670a89cea02aca51324f6a1b0bd14bf0710a
Author: kinuko <kinuko@chromium.org>
Date: Sat Mar 12 04:45:55 2016

Fire visibilitychange event on unload (behind the flag)

Per recent spec change:
http://w3c.github.io/page-visibility/#reacting-to-visibility-changes

BUG= 554834 
TEST=fast/events/page-visibility-iframe-unload.html
     fast/events/page-visibility-unload.html

Review URL: https://codereview.chromium.org/1778753003

Cr-Commit-Position: refs/heads/master@{#380848}

[modify] https://crrev.com/8909670a89cea02aca51324f6a1b0bd14bf0710a/third_party/WebKit/LayoutTests/fast/events/page-visibility-iframe-delete-test.html
[add] https://crrev.com/8909670a89cea02aca51324f6a1b0bd14bf0710a/third_party/WebKit/LayoutTests/fast/events/page-visibility-iframe-unload-expected.txt
[add] https://crrev.com/8909670a89cea02aca51324f6a1b0bd14bf0710a/third_party/WebKit/LayoutTests/fast/events/page-visibility-iframe-unload.html
[add] https://crrev.com/8909670a89cea02aca51324f6a1b0bd14bf0710a/third_party/WebKit/LayoutTests/fast/events/page-visibility-unload-expected.txt
[add] https://crrev.com/8909670a89cea02aca51324f6a1b0bd14bf0710a/third_party/WebKit/LayoutTests/fast/events/page-visibility-unload.html
[rename] https://crrev.com/8909670a89cea02aca51324f6a1b0bd14bf0710a/third_party/WebKit/LayoutTests/fast/events/resources/page-visibility-iframe-with-subframes.html
[modify] https://crrev.com/8909670a89cea02aca51324f6a1b0bd14bf0710a/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/8909670a89cea02aca51324f6a1b0bd14bf0710a/third_party/WebKit/Source/core/dom/Document.h
[modify] https://crrev.com/8909670a89cea02aca51324f6a1b0bd14bf0710a/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in
[modify] https://crrev.com/8909670a89cea02aca51324f6a1b0bd14bf0710a/third_party/WebKit/Source/web/ChromeClientImpl.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 5 2016

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

commit 26f8cbe616e35ac6839970e9b52eb35405f61685
Author: kinuko <kinuko@chromium.org>
Date: Tue Apr 05 05:00:30 2016

Fire visibilitychange on page unloading when experimental flag is on

(So that more people can easily start testing this)

BUG= 554834 

Review URL: https://codereview.chromium.org/1858473002

Cr-Commit-Position: refs/heads/master@{#385106}

[modify] https://crrev.com/26f8cbe616e35ac6839970e9b52eb35405f61685/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in

Related bug report...

"If closing the tab causes the renderer to go away, then the visibility event is not triggered": https://github.com/w3c/beacon/issues/31#issuecomment-219780783


Comment 6 by kinuko@chromium.org, Jun 17 2016

Some recent findings reg: the current impl and behavior (some of these have been discussed in a private thread, but let me note these here too):

- Event dispatch timing: On aura platforms (i.e. on Linux and Windows) we dispatch visibility change event a bit earlier, i.e. before we dispatch pagehide event, while we dispatch this after pagehide on OS X.

- Event is not triggered when the renderer going away issue (mentioned in #5): for now I'm unable to repro the issue (using example.com, beacon and chrome://net-internals) on Linux and Mac OS X.  I responded on the github issue to get more feedback
> - Event dispatch timing: On aura platforms (i.e. on Linux and Windows) we dispatch visibility change event a bit earlier, i.e. before we dispatch pagehide event, while we dispatch this after pagehide on OS X.

Technically, that violates the spec [1], but not sure if that's a big deal..

[1] https://html.spec.whatwg.org/#unload-a-document
Labels: Hotlist-PerformanceAPIs

Comment 9 by kinuko@chromium.org, Jul 19 2016

Blocking: 594049
Cc: panicker@chromium.org
kinuko@: any updates on your end? Any reason why we can't merge this for M56?

Comment 11 by ojan@chromium.org, Oct 24 2016

We were blocking on making it so that the visibilitychange event can fire after the iframe has been removed from the DOM. I think we're probably letting perfect be the enemy of good enough though and we should just ship visibilitychange that fires at the same time as unload. Any objections?
Big +1 from me. We should file a separate bug for the iframe case.
Sorry for being slow to make progress / update status etc on this one.  I will be starting a thread for intent-to-ship to see if we could agree on shipping this as is or what we might still want to consider a real blocker if not.
Filed a separate bug for iframe situation ( issue 636833 ).  Meanwhile I'm going to prepare a intent to ship email and a patch.
> Filed a separate bug for iframe situation

Sorry, wrong link-- not 636833, issue 663276 the one I filed for iframe issue.
@kinuko: awesome, thank you. Getting this patch out will be a nice developer annoyance / platform health fix!
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 15 2016

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

commit d3b76124cc401add44a97321c5609672851fd8af
Author: kinuko <kinuko@chromium.org>
Date: Tue Nov 15 15:20:41 2016

Ship: Fire visibilitychange event on document unloading

Intent to ship thread: https://groups.google.com/a/chromium.org/d/msg/blink-dev/sTJ0OL9QJx8/DAXDOrIqAQAJ

BUG= 554834 

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

[modify] https://crrev.com/d3b76124cc401add44a97321c5609672851fd8af/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in

Comment 18 by ojan@chromium.org, Dec 12 2016

Blocking: 154321
NextAction: 2017-03-01
This has been enabled since M56, as of r432181.  Will remove runtime flag unless we get any complaints.
Project Member

Comment 20 by bugdroid1@chromium.org, May 20 2017

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

commit 252214d2c6d64400632711b960099c3ffb2333c2
Author: kinuko <kinuko@chromium.org>
Date: Sat May 20 20:22:06 2017

Remove visibilityChangeOnUnload runtime flag (as it's shipped)

Feature has shipped in M56 and two milestones have passed, we should
be able to remove the runtime flag now.

BUG= 554834 

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

[modify] https://crrev.com/252214d2c6d64400632711b960099c3ffb2333c2/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/252214d2c6d64400632711b960099c3ffb2333c2/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5

Status: Fixed (was: Assigned)

Sign in to add a comment