are image pings in unload actually supported?

674 views
Skip to first unread message

Fergal Daly

unread,
Dec 15, 2023, 10:01:48 AM12/15/23
to net-dev, Nonoka Muraki
TL;DR do we ensure that the network request for an image object created in unload will actually be kept alive?

This CL is moving the image-ping test to use pagehide instead of unload. This test creates an image in the unload handler and tests that the network request to fetch the image is received by the server.

From the original bug:
There's consensus for allowing images loaded in unload handlers to outlive the page, in order to maintain compatibility with IE.  This hack is used by a bunch of ad networks for tracking purposes.  Since the image doesn't currently outlive the page, they resort to simulating sleep() in JavaScript by busy looping.  By letting the image load outlive the page, the goal is that web developers won't resort to such hacks which hurt the user.

It looks like in the bug some code was changed to ensure this. 

However, since the test is being renamed it is now subjected to the flaky test detector for new tests, which says that it is flaky. I already removed 1 source of flakiness with this CL. This has resulted in green runs on windows and linux but mac is still failing. I can't think of any other source of flakiness except that we just don't truly support this use-case and that sometimes the network request doesn't make it.

Does the code still exist? Was it dropped when sendBeacon/keep-alive became an option? Should we actually be testing this?

F

Tsuyoshi Horo

unread,
Dec 18, 2023, 9:51:59 AM12/18/23
to Fergal Daly, net-dev, Nonoka Muraki, Nate Chapin
The logic seems to exist in ImageLoader::DoUpdateFromElement(). And according to the UseCcounter ImageLoadAtDismissalEvent, it is used in 4% of all page loads.

I'm not sure how reliable this logic is.
I think japhet@ knows the details.


--
You received this message because you are subscribed to the Google Groups "net-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to net-dev+u...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/net-dev/CAAozHLnORU9REDo5t4MY%2BHrsr2Fmf%3DR6eJQvy1cx%2BBomG09HKA%40mail.gmail.com.

Fergal Daly

unread,
Dec 18, 2023, 9:51:59 AM12/18/23
to Tsuyoshi Horo, net-dev, Nonoka Muraki, Nate Chapin
On Mon, 18 Dec 2023 at 11:37, Tsuyoshi Horo <ho...@chromium.org> wrote:
The logic seems to exist in ImageLoader::DoUpdateFromElement(). And according to the UseCcounter ImageLoadAtDismissalEvent, it is used in 4% of all page loads.

Thanks. My reading of that code is that pagehide should be equivalent to unload for these purposes since `PageDismissalEventBeingDispatched() != Document::kNoDismissal` will still be true

I'm not sure how reliable this logic is.
I think japhet@ knows the details.

Here is the new flake detector firing after renaming the test. So the test seems to be currently slightly flaky. It's a bit surprising because this is all same-site, so I don't think the renderer is dying. The network stuff should be fully reliable (assuming it's hitting that codepath and firing off a keep-alive request),

F

Fergal Daly

unread,
Dec 19, 2023, 10:12:29 AM12/19/23
to Tsuyoshi Horo, Ming-Ying Chung, net-dev, Nonoka Muraki, Nate Chapin
On Mon, 18 Dec 2023 at 14:10, Fergal Daly <fer...@google.com> wrote:
On Mon, 18 Dec 2023 at 11:37, Tsuyoshi Horo <ho...@chromium.org> wrote:
The logic seems to exist in ImageLoader::DoUpdateFromElement(). And according to the UseCcounter ImageLoadAtDismissalEvent, it is used in 4% of all page loads.

Thanks. My reading of that code is that pagehide should be equivalent to unload for these purposes since `PageDismissalEventBeingDispatched() != Document::kNoDismissal` will still be true

I'm not sure how reliable this logic is.
I think japhet@ knows the details.

Here is the new flake detector firing after renaming the test. So the test seems to be currently slightly flaky. It's a bit surprising because this is all same-site, so I don't think the renderer is dying. The network stuff should be fully reliable (assuming it's hitting that codepath and firing off a keep-alive request),

Even more interesting, here is the new flake detector not firing (same CL PS2, 2 runs on win/mac/linux) after I enabled KeepAliveInBrowserMigration.

That's pretty strong evidence that the flakiness is just inherent flakiness in keep-alive, although it's surprising given that there's really not much going on in this test and no reason for it to be unreliable,

F

Nate Chapin

unread,
Dec 19, 2023, 12:34:07 PM12/19/23
to Tsuyoshi Horo, Fergal Daly, net-dev, Nonoka Muraki
Sorry, I'm not going to be much use here. I haven't really looked at this logic since I implemented it in 2010, and I haven't been paying attention to how reliable it is in practice, or how necessary it actually is on the modern web.

Fergal Daly

unread,
Dec 21, 2023, 11:50:42 AM12/21/23
to Nate Chapin, Tsuyoshi Horo, net-dev, Nonoka Muraki
OK, thanks. My conclusion at this point is that with KeepAliveInBrowserMigration enabled this test no longer flakes, so I am going to force-submit the CL that migrates it to pagehide,

F
Reply all
Reply to author
Forward
0 new messages