tracking installs per web contents (Web install API)

49 views
Skip to first unread message

Lia Hiscock

unread,
Apr 18, 2025, 2:42:29 PMApr 18
to pwa-dev
Hey folks/Dan!

I was reviewing the notes from Dan and Howard's sync last week, and had a couple of follow up questions.

1. Was/is there a reason for having the install dialogs be non-light dismiss? Naively, it seems like that would fix the entire "don't show multiple dialogs for the same tab" issue.

Assuming we can't make the dialogs light dismiss....

"if the ml tracker isn't there, then create it and attach to the current process (forward to command?)"
2. Confirming that this means background installs should still register as coming from the current web contents? (Makes sense to me, as it will be the current tab that's showing the background install dialog)

"this MIGHT be implemented in a couple places - dialog coordinator? But the main place this functionality is working is the ML Promotion Tracker (or something like that)"
3. There was mention of a "dialog coordinator" -- could you point me to the class for this? I saw isolated_web_app_installer_coordinator, but this didn't seem correct.

"It would be good to.... refactor this a little to make it more deliberate
  • class CurrentInstallProcess : public WebContentUSerData - or BrowserUserData? IDK
  • inside of there we can put the ML tracker
  • IDK if we make this 'early' enough???"

4. General clarification about the suggested way forward -- it seems like we're generally supportive of a new class. Could you expand on the concern about making this "early" enough? Is this referencing the fact that we'd need to register the install earlier than when we reached the WebInstallServiceImpl in the browser process?

Thanks!
Lia

Daniel Murphy

unread,
Apr 21, 2025, 1:42:16 PMApr 21
to Lia Hiscock, Dibyajyoti Pal, pwa-dev
I haven't dived into the code too much here and I expect you'll have more knowledge about the state & our options. I'm assuming you've been doing prototyping around here etc.

Likely too much here, but some thoughts:

1. Was/is there a reason for having the install dialogs be non-light dismiss?

I wasn't aware of 'light-dismiss' -does this mean that if one is showing it gets dismissed if we show another? What kind of work would it take to get this to work? I know we test for installation blockage on another trigger, so you would have to change some tests I think.

It would be good to summarize what this change means for the user - I'm guessing this means that.... at most this would mean that if you go to the 3-dot menu when the install dialog is showing, and click install, it'll just dismiss & reshow?

No matter what, we still need a way to have this dialog dismissed before or during the command if:
  • The browser tab changes
  • The web contents navigates

2. Confirming that this means background installs should still register as coming from the current web contents?

For background installed - yes I believe so, as this is more related to the browser/tab that the UX is showing up for, and not the web contents that is being installed.

3. There was mention of a "dialog coordinator" -- could you point me to the class for this? I saw isolated_web_app_installer_coordinator, but this didn't seem correct.

This... perhaps this doesn't exist anymore. I thought @Dibyajyoti Pal made something here at some point but maybe we refactored it away.

4. General clarification about the suggested way forward -- it seems like we're generally supportive of a new class. Could you expand on the concern about making this "early" enough? Is this referencing the fact that we'd need to register the install earlier than when we reached the WebInstallServiceImpl in the browser process?

I don't remember what I meant by that. I also don't know the 'right' solution here, and so this would be something that you would have to poke at. Perhaps you can 'hack' it in with the existing ml tracker stuff.

Unstructured brain dump, but with the caveat that maybe none of this is needed - this just seems like a confusing area that can be unified into a single logic place for this - a WebAppDialogCoordinator that lives in... chrome/browser/ui? And would have to have an interface available for components/webapps/ to see, perhaps piping through the WebAppClient like other stuff...
  • (probably semantically most correct) Create a BrowserUserData class that's like WebAppDialogCoordinator.
    • It would listen to the browser tab observer and dismiss dialog when tab changes. Also other stuff like if the page navigates, etc?
    • Maybe methods like:
      • Maybe methods likestatic WebAppDialogCoordinator::GetForBrowser()
      • WebAppDialogCoordinator::is_web_app_dialog_showing()
      • WebAppDialogCoordinator::DismissShownDialog()
      • WebAppDialogCoordinator::RegisterMLTrackerForCurrentPage
        • it would have to be a WebContentsObserver to reset this on NavigationStarted etc.
      • (called from dialogs / install processes)
      • WebAppDialogCoordinator::OnDialogWillShow(base::WeakPtr<WebAppCoordinatorDialogDelegate>)
      • WebAppDialogCoordinator::OnDialogClosed -> maybe can be done is a nice observer way.... not wure
  • other option - do something like that but as a WebContentsUserData?
  • In some ways this is pulling out existing code from AppBannerManager into a separate place, which is always nice.




--
You received this message because you are subscribed to the Google Groups "pwa-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pwa-dev+u...@chromium.org.
To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/pwa-dev/226063e5-78af-4265-b40d-3e7321da469en%40chromium.org.

Dibyajyoti Pal

unread,
Apr 21, 2025, 1:48:28 PMApr 21
to Daniel Murphy, Lia Hiscock, pwa-dev
Hi Lia,


> There was mention of a "dialog coordinator" -- could you point me to the class for this? I saw isolated_web_app_installer_coordinator, but this didn't seem correct.

We no longer have a coordinator, but rather a WebAppInstallDialogDelegate that controls the behavior for the 3 types of install dialogs that we have (DIY, Simple and Detailed). The way we keep track of the type of dialog it interacts with is via this enum. This is a very loose implementation of the MVC architecture.

Hope this helps!

Thanks!
Regards,
Dibya
Message has been deleted

Lia Hiscock

unread,
Apr 25, 2025, 12:22:21 PMApr 25
to pwa-dev, Dibyajyoti Pal, Lia Hiscock, pwa-dev, Daniel Murphy
Thanks Dan and Dibya for the thoughts! Apologies for the late reply, I've been focused on some unrelated things these past 2 weeks, but am hoping to be able to deep dive into this more next week.

Thanks :)
Lia

Message has been deleted
Message has been deleted

Lia Hiscock

unread,
Apr 30, 2025, 3:05:24 PMApr 30
to pwa-dev, Dibyajyoti Pal, Daniel Murphy
Hey Dan! 

Thanks for the patience. "Light dismiss" means we monitor for mouse input and close the dialog on any click outside the bounds of the dialog. This effectively covers tab switching, window losing focus, clicking on the ... menu, etc. In Edge, our install dialog is a bubble dialog delegate view, and we implement ui::EventObserver to listen for the mouse events. I spent some time trying to tack this onto the WebAppInstallDialogDelegate that Dibya mentioned, but ultimately decided it was incompatible with the ModalDialog model that your dialogs use, and would likely require more extensive changes than we'd want to make.

However, your confirmation that background installs should still register as coming from the current web contents led me down a different path, and I was able to get a working prototype that didn't require changes to the ML tracker. I believe it's effectively this approach that you and Howard discussed -- 
  • Naive possibility here is that the web install API just needs to check & make this on the initial JS call, on the browser side?

      • if the ml tracker isn't there, then create it and attach to the current process (forward to command?)

      • if it IS there, then abort the install.


    We think this change makes sense in the context of web install, as the web contents we register the install on is also used by the DialogDelegate to anchor the dialog to. It also seems like a minimally disruptive change that will be fairly straightforward to update whenever the ML tracker ends up being removed.

    I've tagged you in a couple places to confirm some assumptions, but other than that I'd love to get your general thoughts!

    Thanks!
    Lia

    Daniel Murphy

    unread,
    Apr 30, 2025, 5:26:23 PMApr 30
    to Lia Hiscock, pwa-dev, Dibyajyoti Pal
    I think that approach seems reasonable.

    Message has been deleted

    Daniel Murphy

    unread,
    May 5, 2025, 2:06:54 PMMay 5
    to Lia Hiscock, pwa-dev, Dibyajyoti Pal
    (sorry for the duplicate email everyone, I think I solved our spam auto-blocking problem, this was a duplicate send from that. My fault)

    On Mon, May 5, 2025 at 11:05 AM 'Lia Hiscock' via pwa-dev <pwa...@chromium.org> wrote:
    Hey Dan!

    Thanks for the patience. To answer your question, "light dismiss" means we monitor for mouse input and close the dialog if we receive any clicks outside the bounds of the dialog. This effectively handles tab switches, window losing focus, clicking the ... menu, etc. In Edge, our install dialog is a bubble dialog delegate view and it implements event_observer.h to listen for mouse events. I played around a bit trying to hack this into the WebAppDialogDelegate, but came to the conclusion that it's not very compatible with the ModalDialog model, and would likely require changing the dialog model.

    Your confirmation about background installs registering on the current web contents led me down a different path, and I was able to prototype a working solution with the ML tracker, that's pretty much this approach you and Howard discussed --
    • Naive possibility here is that the web install API just needs to check & make this on the initial JS call, on the browser side?

      • if the ml tracker isn't there, then create it and attach to the current process (forward to command?)

      • if it IS there, then abort the install.


    I tagged you in a couple places that I had confirmation questions, but I'd love to get your initial thoughts! We're thinking this is a minimally disruptive change that makes sense in the context of web install, and also won't require too much updating when the tracker gets removed.

    Thanks!
    Lia

    On Friday, April 25, 2025 at 9:22:21 AM UTC-7 Lia Hiscock wrote:
    Reply all
    Reply to author
    Forward
    0 new messages