prebid / Prebid.js

Setup and manage header bidding advertising partners without writing code or confusing line items. Prebid.js is open source and free.
https://docs.prebid.org
Apache License 2.0
1.27k stars 2.05k forks source link

Refreshing ads on single page applications causes excessive memory usage. #10880

Open 12q1 opened 8 months ago

12q1 commented 8 months ago

Type of issue

Question

Description

When creatives are given access to the top window object they insert third party scripts and create additional DOM nodes. On traditional static HTML pages this is almost unpercievable but when combined with ad refreshes on single page applications these references are not cleaned up and can heavily impact performance.

Steps to reproduce

Can be simulated on single page applications by navigating from page to page or waiting for multiple refreshes.

Test page

Issue can be observed on: https://www.puna.nl

Expected results

Expect creatives to be unloaded gracefully after refreshing and remove references/DOM nodes from the page.

Actual results

Over time the memory (Read Total JS Heap) increases throughout the user journey. In worst case scenarios the memory usage can become so high that it locks up the client's browser.

image

Platform details

All web integrations regardless of browser/OS/build version.

Other information

Seems to be a known issue:

We are curious if there has been any changes to Prebid's recommendation to force safe frames to prevent creatives from breaking out of their sandbox and accessing the top window. Some of our rich media partners prefer to work without safe frames so this creates a problem when trying to support their integrations.

Thanks.

patmmccann commented 8 months ago

are you taking advantage of #10308

12q1 commented 8 months ago

are you taking advantage of #10308

@patmmccann yes, our prebid config object currently contains the following settings:

pbjs.setConfig({
        minBidCacheTTL: 30,
        eventHistoryTTL: 30,
});

Would you suggest putting these options even lower? We did see some minor improvement from these setting but the bulk of the memory usage still seems to be caused by creatives leaving references (eg. event listeners) and DOM nodes on the page.

patmmccann commented 8 months ago

Are you seeing an issue where safari mobile just refreshes pages?

patmmccann commented 8 months ago

Monkey patching insertelement, add listener etc, you could tag the element with a creative id and clean them up later. This is quite onerous. It seems a community push towards safeframe is likely a better solution. We're likely able to solve some cleanup issues with very specific examples. Maybe let's open more issues of this nature detailing some of the remnant items

12q1 commented 8 months ago

@patmmccann thanks for the update, can't say we've noticed the safari issue but will take a look.

Regarding pushing for safeframes I couldn't agree more but as it stands our partners are not willing to relinquish their access to the top window. Have to balance the business requirements against the risk here.

jpm commented 8 months ago

Are you seeing an issue where safari mobile just refreshes pages?

We have noticed the page refresh issue both on Safari/iOS as well as Chrome/iOS. We are assuming that this is because both browsers use WebKit, the same underlying web browser engine.

It's been a difficult issue to troubleshoot, as Apple doesn't seem interested in dealing with the problem, and other vendors don't have a good way to avoid it.