Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generateBid function needs to be global function #444

Closed
k-o-ta opened this issue Feb 1, 2023 · 6 comments
Closed

generateBid function needs to be global function #444

k-o-ta opened this issue Feb 1, 2023 · 6 comments
Labels
compat concern likely to be a breaking change for developers

Comments

@k-o-ta
Copy link
Contributor

k-o-ta commented Feb 1, 2023

Hi,

GenerateBid function needs to be global function under the current specification.
This restriction conflict with ES modules system.
I predict that when the bidding logic becomes more complicated, I will want to use ES modules.

Here are some rough ideas for this, but I'm sorry for I don't know if this is feasible.

  1. Load a biddingLogic file from biddingLoginUrl as type="module" and import generateBidfunction.
  2. Call addEventListner(message) and postMessage(bid) in a biddingLogic file and a browser calls postMessage(interestGroup, auctionSignals, perBuyerSignals, trustedBiddingSignals, browserSignal) to Ad auction worklet
    a. In this case, buyers need to bundle ES modules files into one script file using tools such as webpack.
@domfarolino
Copy link
Collaborator

I think we'd really need to see if there is appetite to make FLEDGE worklets run module scripts instead of classic scripts (as it stands now, the Chromium implementation runs classic scripts, and I'm not aware of appetite for changing things at the moment). My understanding is that the worklets don't have network access, so I'm not really sure I understand how them fetching a module graph would really work. Sorry, I don't fully understand the list of options you mentioned.

@k-o-ta
Copy link
Contributor Author

k-o-ta commented Mar 23, 2023

Thank you for reply.
I'm sorry that my explanation wasn't clear enough.

This simply meant that the file defining the generateBid function can be imported modules. (I did not know the term classic script. Thanks for letting me know.)

biddingLogicURL file

import {getBid} from "./module";

function generateBid(group, auctionSignals, perBuyerSignals,
                     trustedBiddingSignals, browserSignals)  {
      return {
        bid: getBid(group),
        ad: { adName: "adName"},
        render: "https://example.com/creative",
    }
}

module.js

export const getBid = (igName): number => {
    // complicated calculation logic
    return 300
}

This approach is using a bundling tool such as webpack to enable file splitting without network connections for imports.

The main thread of the browser and the auction worklet communicate using the postMessage function, similar to how the main thread of the browser and web workers communicate using the postMessage function.

biddingLogicURL file

import {getBid} from "./module";

onmessage = function (group, auctionSignals, perBuyerSignals, trustedBiddingSignals, browserSignals)  {
      postMessage(
        {
          bid: getBid(group),
          ad: { adName: "adName"},
          render: "https://example.com/creative",
        }        
      );
}

The browser automatically posts a message to auctionWorklet when running the runAdAuction. This method does not require defining the generateBid function. The reason I came up with this method is that when using webpack to use imports, webpack automatically removes global functions. There may be other methods such as finding ways for webpack to not remove global functions or using other bundling tools, but I haven't investigated all of them.

@brusshamilton
Copy link
Contributor

There would be a significant overhead if we switched to postMessage for communication. The FLEDGE worklet environment is fundamentally different from other web worklet environments due to the amount of isolation and limited lifetime applied to its worklets.

For webpack, you should be able to manipulate the global object directly to add a function:

globalThis.generateBid = function(...) {...}

@k-o-ta
Copy link
Contributor Author

k-o-ta commented Mar 23, 2023

For webpack, you should be able to manipulate the global object directly to add a function:

globalThis.generateBid = function(...) {...}

Thank you for information. I'll try this!

@k-o-ta
Copy link
Contributor Author

k-o-ta commented Mar 23, 2023

I tried it with the following simple code and it worked. Thanks @brusshamilton .
webpack.config.js

module.exports = {
  mode: "development",
  entry: {
    main: './main.js',
  },
  output: {
    path: `${__dirname}/dist`,

  },
};

main.js

import { getCPA } from './module'

globalThis.generateBid = function(group, auctionSignals, perBuyerSignals, trustedBiddingSignals, browserSignals) {
    return {
        bid: getCPA(group.name),
        ad: {
            adName: "adName"
        },
        render: "https://example.com/creative",
    }
}


globalThis.reportWin = function(auctionSignals, perBuyerSignals, sellerSignals, browserSignals) {
    console.log(browserSignals)
}

@JensenPaul JensenPaul added the compat concern likely to be a breaking change for developers label Jun 27, 2023
@domfarolino
Copy link
Collaborator

Thanks for starting this thread @k-o-ta. Given: (a) the super tight constraints on Protected Audience script runners (i.e., no network access among others), (b) the only motivation in our well-over-1-year of Origin Trialing for considering module-adjacent import behavior being a preprocessing tooling concern with webpack, and (c) the fact that this concern was reasonably addressed above, I think it is safe to close this.

As script runners evolve in the future we could re-hash the discussion provided sufficient community appetite. Thanks!

@domfarolino domfarolino closed this as not planned Won't fix, can't repro, duplicate, stale Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat concern likely to be a breaking change for developers
Projects
None yet
Development

No branches or pull requests

4 participants