Re: ScopedFrame/AgentPauser

[Charlie Reis]

[+site-isolation-dev, dcheng, clamy]

At a high level, I'm excited to see work to improve ScopedPagePauser to be at the granularity of Browsing Context Groups, which is useful whether or not ProcessPerSite mode is used more often or not.  (I think there are some big caveats in the experiment results that we still need to discuss before decisions about ProcessPerSite are made.)

blink::Page::RelatedPages() is the current approximation of Browsing Context Groups in Blink, and ahemery@ added Page::BrowsingContextGroupToken() as well in r1145881 for the COOP:RP work.  He filed https://crbug.com/1464622 to create an actual BrowsingContextGroup class, as an improvement over tracking the tokens.

I don't think you want to use SiteInstanceGroup or AgentSchedulingGroup for any of this-- both of those can be tuned and change their behavior over time, as haraken@ notes.  SiteInstance is also not the right granularity-- DevTools needs to pause everything in the Browsing Context Group in a given process, not just the SiteInstance.

So, we should be aiming to pause all pages with the same BrowsingContextGroupToken(), which I understand to be the pages in blink::Page::RelatedPages().

Charlie

On Tue, Aug 8, 2023 at 8:42 AM Kentaro Hara <har...@chromium.org> wrote:

Thanks Dominic! We're getting close :)

I agree that blink::Page::RelatedPages() will *mostly* give us what we need now *if* an AgentGroupScheduler is 1:1 with a SiteInstanceGroup.

A few things:

- An AgentGroupScheduler is 1:1 with a SiteInstanceGroup when the MBI mode is on. Currently the MBI mode is off, and the AgentGroupScheduler is created per renderer process (so blink::Page::RelatedPages() is equal to blink::Page::OrdinaryPages()). Is my understanding correct?

- Enabling the MBI mode will require more engineering work. In the meantime, I think we can just implement blink::Page::RelatedPages() so that it returns the PageGroup regardless of whether the MBI mode is on or off. It should be easy. This unblocks us.

- I wrote "mostly" because I think that blink::Page::RelatedPages() should return pages in the SiteInstance, not the SiteInstanceGroup. The SiteInstanceGroup is a performance isolation unit and may contain an arbitrary number of SiteInstances in the BrowsingContextGroup. If blink::Page::RelatedPages() returns pages in the SiteInstanceGroup, what SiteInstances are paused by the ScopedPagePauser depends on the site isolation policy. To make the behavior deterministic, blink::Page::RelatedPages() should return pages in the SiteInstance (not the SiteInstanceGroup). It won't be hard to implement this.

Overall my proposal is: Regardless of whether the MBI mode is enabled or not, implement blink::Page::PagesInTheSameSiteInstance(). And make the ScopedPagePauser use it.

Does it make sense? :)

On Tue, Aug 8, 2023 at 11:19 PM Dominic Farolino <d...@chromium.org> wrote:

Doesn't AgentGroupScheduler already cover multiple blink::Pages in the same browsing context group? In the same-origin opener case that Charlie mentioned, I believe the newly-created blink::Page/blink::WebView is governed by the opener's AgentGroupScheduler, which would mean we already have the primitives required to achieve this. At least I'm pretty sure that was the motivation with MBI. Consider this flow:

FrameTree::FindOrCreateFrameForNavigation()

  old_page->CreateNewWindow()

    ChromeClientImpl::CreateWindowDelegate()

      (opener) RenderFrameImpl::CreateNewWindow()

        AgentSchedulingGroup::CreateWebView() --> blink::WebView::Create(..., agent_group_scheduler(), ...)

I'll admit this is just a cursory code search, but it's motivated by my hunch that trying to introduce something like PageGroup, specifically for scheduling, is redundant over the existing MBI primitives. And separately:

Would it be hard for Blink to get a set of Pages in the same BrowsingContextGroup? Let's call it a PageGroup.

I think this might already be covered by the concept of "related" pages in Blink? See blink::Page::RelatedPages(), which consults a "circular, double-linked list of pages that are related to the current browsing context". We should confirm whether that gives us all top-level browsing contexts in a particular BCG, but I think it might.

On Tue, Aug 8, 2023 at 4:36 AM Kentaro Hara <har...@chromium.org> wrote:

@Kouhei Ueno  pointed out that Charlie is already discussing this issue in this comment.

Charlie wrote:

I don't think changing ScopedPagePauser from process-wide to page or frame specific is a viable option.  There could be other pages in the same BrowsingContextGroup and process (e.g., a same-origin opener) which must also be paused, or else the opener can interact with a page that is stuck on a breakpoint.

In Blink terms, I think this means it would need to be PageGroup-wide, which is the representation of a BrowsingContextGroup in a given process.  (In browser process terms, DevTools could probably allow a different BrowsingInstance in the same renderer process to keep running during a breakpoint.)

Would it be hard for Blink to get a set of Pages in the same BrowsingContextGroup? Let's call it a PageGroup.

The browser process has the information. For example, can the browser process send an UnguessableToken of the BrowsingContextGroup when creating a Page in Blink? Then Blink can create the PageGroup and update ScopedPagePauser::SetPaused() to:

for (auto* page : page_group) {

  page->SetPaused(true);

}

As Yukishiino mentioned, this will fix the problems of OpenJavaScriptDialog(), Print() etc :)

On Tue, Aug 8, 2023 at 5:30 PM Yuki Shiino <yukis...@chromium.org> wrote:

+cc: thestig@ just FYI,

Probably this will unblock https://crbug.com/956832 and https://crbug.com/1316315 . A long wanted feature.

2023年8月8日(火) 16:50 Kentaro Hara <har...@chromium.org>:

Hi

Context: Recently bashi@ experimented with ProcessPerSite (TL;DR: Let multiple same-origin tabs share one renderer process) and it produced promising results to improve the global LCP. There are a couple of blockers to make ProcessPerSite launchable and we are resolving it one by one.

Problem: One blocker is ScopedPagePauser used by DevTools (see this bug for the detail). DevTools uses the ScopedPagePauser to pause the execution and debug. This causes a problem with ProcessPerSite because the ScopedPagePauser is a singleton in the renderer process and pauses all pages inside the process. e.g., when you pause an execution of example.com in one tab, example.com in the second tab is paused too. In theory, this is an existing bug because Chrome may put multiple tabs in one renderer process when it exceeds the soft process limit (see the "Process Reuse" section of this doc). However, it is rare enough. ProcessPerSite makes the problem happen much more often.

Proposed solution: MBI implemented a per-frame scheduler / per-agent scheduler. My proposal is to implement ScopedFramePauser / ScopedAgentPauser, which pauses tasks of the specific frames / agents. One caveat is that not 100% of the tasks go through the per-frame / per-agent schedulers (some of the tasks go through the global page scheduler -- we should fix them) but I think the coverage is close to 100%.

Will it solve the problem of DevTools? :)

Also, I think we should replace other usages of ScopedPagePauser (e.g., OpenJavaScriptDialog() and Print()) with ScopedFrame/AgentPauser.

--

Kentaro Hara, Tokyo

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jyDoc2r4-JyZtSUKqPvhnTG%3D3TkC6wF%3D%3DqPXRuZH9TUTw%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAN0uC_QSs6Yup1CqW1BvN6aBXx6PLVUh%2BqZZebZMQWjYGmTDBw%40mail.gmail.com.

--

Kentaro Hara, Tokyo

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jy9yRGAgQNUzuqZ_Gd8YrSEX3VChyn488XBpht24rHkFw%40mail.gmail.com.

--

Kentaro Hara, Tokyo