Integrating Fenced Frames with MPArch long-tail feature development

[Dominic Farolino]

Prerendering is paving the path as the first and main MPArch consumer, which has revealed various assumptions in the codebase that are incompatible with MPArch. While working on Fenced Frames, I've been running into similar long-tail related problems that prerendering is facing, and I have some questions about how to integrate Fenced Frames (and more generally other MPArch consumers) into this whole space.

One of the first issues I ran into was the PageLoadMetricsObserver DCHECK'ing [cs] when navigating a <fencedframe> hosted by MPArch. This was fixed for prerendering by this CL, however I noticed that the fix did not work for Fenced Frames out-of-the-box because our WIP does not add a new RFHI LifecycleState enum value.

My understanding is that in order to fix problems like this for fenced frames, it is necessary to add:

1. A new RenderFrameHostImpl::LifecycleStateImpl enum value (presumably one for RFH and for RFHI)
1. This was made clear by altimin@ given some discussion in the internal MPArch channel
2. A new FrameTree::Type enum value for fenced frames as well
1. This was mentioned by sreejakshetty@ also in the internal MPArch channel

I'm happy to add a new kFencedFrames value to each of these enums, however I want to make sure I'm doing so responsibly. My initial interest is to suppress bugs in e.g., PageLoadMetricsObserver and ContentFaviconDriver for fenced frames in the same way we do for prerendering, but I'm worried about introducing side effects.

There is certainly code that conditionally allows some behavior for kActive RenderFrameHostImpls and kPrimary FrameTrees, that we disallow for prerenders via the kPrerendering value among other things. I imagine we'll want a similar set of restrictions for fenced frames, but I'd like to get some MPArch folks' opinion on how to introduce a kFencedFrames value and protect against inadvertently losing a bunch of kActive-esque behavior that we may actually want for fenced frames. The only mitigation I can think of is doing a full audit of all kActive and kPrerendering usages, and ensuring that we'd have the right behavior for kFencedFrames in each place. This seems tedious and dependent on domain-specific knowledge in some places, so I just want to see if folks have other guidance here for MPArch-consuming projects. The alternative seems to be: just introduce the value, and hope it doesn't break important things. The only way to verify that <fencedframe> is truly correct in this case seems to be to write a test for every single web platform feature being used in a <fencedframe>, which certainly isn't scalable :)

More generally, I imagine we'll have two types of MPArch consumers in the long-run:

1. Fully hidden consumers with some set of needs and restrictions similar to kPrerendering today
2. Nested-and-visible consumers with perhaps a different set of needs and restrictions. This is similar to kPrerendering, but may look a little more like kActive in some places, since we'll support rendering/input and act more like a subframe than a truly invisible page

At the moment I'm not sure what the difference in needs might be between the two, but any work that we can do for Fenced Frames to help generalize this pattern seems like a good idea if my perspective is at all correct.

Thanks,

Dom

[Kentaro Hara]

1. A new RenderFrameHostImpl::LifecycleStateImpl enum value (presumably one for RFH and for RFHI)

1. A new FrameTree::Type enum value for fenced frames as well

I just want to make sure that states in RenderFrameHostImpl::LifecycleStateImpl are exclusive. Are kActive, kPrerender, kInBackForwardCache and kFencedFrames exclusive from each other?

If so, adding new types makes sense to me. Otherwise, we need to rethink :)

 

--
You received this message because you are subscribed to the Google Groups "navigation-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to navigation-de...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/navigation-dev/CAP-uykDm-2CgZcxgRqhbH4Dfm0vdCc4BMakdYdMQu%3D5xKm-iow%40mail.gmail.com.

--

Kentaro Hara, Tokyo

[Dominic Farolino]

They are exclusive in the sense that the RenderFrameHostImpl can only be in one of those states at a time. They are not exclusive in the sense that some RenderFrameHostImpl behavior might be allowed for multiple states, and disallowed for other (multiple) states. I guess it is this latter lack of exclusivity that I'm worried about: if we add a new kFencedFrames entry, or any other entries in the future, it at least feels like we'd have to audit all usages of the existing states to see what behavior we want in common with other states etc. Does that make sense?

[Kentaro Hara]

They are exclusive in the sense that the RenderFrameHostImpl can only be in one of those states at a time.

What happens if a page that has a fenced frame is being prerendered or BFcached? What is the state of the fenced frame RFH expected to be?

--

Kentaro Hara, Tokyo

[Dominic Farolino]

This is a great question. Naively I was thinking we'd do what portals would do, which is simply disable BFCache, however if we want to host ads in a <fencedframe> element, then disabling BFCache when a <fencedframe> is present would amount to disabling BFCache for a significant portion of the web, which seems bad. In that case it sounds like Fenced Frames would not be exclusive with respect to at least BFCache, hmmm, this would require more significant design discussions then (I'll bring this up in the MPArch technical discussion weekly meeting in a couple of days).

Regarding prerendering: I don't think I know enough about the prerendering implementation to have a good answer, and frankly I was hoping I wouldn't have to :) . It would depend on a few questions, which perhaps +Matt Falkenhagen could answer:

• For prerendered pages, what subframes are deferred/restricted: cross-origin subframes? same- and cross-origin subframes?
• Where in the code does the above restriction on subframes happen in prerendered pages?
• Does the restriction on subframes happen so early (in Blink for example) that the deferred subframes don't even have RFHIs yet, or is it later, after RFHI creation?
• Is it safe to assume that fenced frames would not have this restriction applied for free? (I can probably answer this myself with the answer to the above question)
• Do RFHIs for subframes in a prerendering FrameTree all have their lifecycle state set to kPrerendering too?
• If the answer here is yes, then we know adding a kFencedFrames value to the LifecycleStateImpl enum will not be trivial, since the states are not exclusive

[Станислав Кучер]

Hello dear developers, please tell me - why i recive all this emails?

С уважением, Кучер Станислав.

Без вирусов. www.avast.com

ср, 14 апр. 2021 г. в 04:30, Kentaro Hara <har...@chromium.org>:

--
To unsubscribe from this group and stop receiving emails from it, send an email to prerender+...@chromium.org.

[Станислав Кучер]

i found)) unsucribed))

С уважением, Кучер Станислав.

Без вирусов. www.avast.com

ср, 14 апр. 2021 г. в 04:30, Kentaro Hara <har...@chromium.org>:

1. A new RenderFrameHostImpl::LifecycleStateImpl enum value (presumably one for RFH and for RFHI)

--
To unsubscribe from this group and stop receiving emails from it, send an email to prerender+...@chromium.org.

[Matt Falkenhagen]

bcc prer...@chromium.org (that list was supposed to be turned down and we've been using navigation-dev so far)

I agree there are tricky issues about LifecycleState and exclusivity. I can respond to the prerendering questions.

2021年4月14日(水) 12:39 Dominic Farolino <d...@chromium.org>:

This is a great question. Naively I was thinking we'd do what portals would do, which is simply disable BFCache, however if we want to host ads in a <fencedframe> element, then disabling BFCache when a <fencedframe> is present would amount to disabling BFCache for a significant portion of the web, which seems bad. In that case it sounds like Fenced Frames would not be exclusive with respect to at least BFCache, hmmm, this would require more significant design discussions then (I'll bring this up in the MPArch technical discussion weekly meeting in a couple of days).

Regarding prerendering: I don't think I know enough about the prerendering implementation to have a good answer, and frankly I was hoping I wouldn't have to :) . It would depend on a few questions, which perhaps +Matt Falkenhagen could answer:

• For prerendered pages, what subframes are deferred/restricted: cross-origin subframes? same- and cross-origin subframes?

Cross-origin subframes.

• Where in the code does the above restriction on subframes happen in prerendered pages?
• Does the restriction on subframes happen so early (in Blink for example) that the deferred subframes don't even have RFHIs yet, or is it later, after RFHI creation?

This is actually being reworked now. It's currently happening as a Navigation Throttle which defers any subframe navigation to an origin other than the one that initiated the prerendered page, but moving it earlier (to Blink) is desirable.

• Is it safe to assume that fenced frames would not have this restriction applied for free? (I can probably answer this myself with the answer to the above question)

I'm not sure now... we currently defer any cross-origin ones. Would this change if we move it to Blink or another timing?

 

• Do RFHIs for subframes in a prerendering FrameTree all have their lifecycle state set to kPrerendering too?

Yes, this is how it works today. I think we set them upon navigation commit here.

[Alexander Timin]

Thanks Dom! 

Overall +1 to adding kFencedFrame both to RFH/RFHI LifecycleState and FrameTreeType from my side.

Some more details:

- Exclusivity: it's indeed an interesting question, especially w.r.t pages with fenced frames entering bfcache.

  My opinion here is that the LifecycleState is the abstraction which allows us to communicate to the //content embedders what a particular document should and should not do.

  Given that the restrictions for bfcache are strictly more strict than restrictions for fenced frames (as the document isn't visible to the user, is frozen and isn't allowed to do pretty much anything),

  so we should set the lifecycle state to such documents to kBackForwardCache (as the embedder shouldn't do anything differently for "fenced frame in bfcache" vs "iframe in bfcache" situations).

  

- Testing: Unfortunately, due to the nature of the problem, we'll need to ensure that pretty much every (non-trivial) web platform feature has a test covering its interaction with fenced frames.

  It's a substantial amount of work, but I don't think we can avoid it (however, I'm hopeful that we should be able to share a lot of things here between bfcache / prerendering / portals & fenced frames here). 

- Re: types of consumers:

  There are indeed various types of MPArch-based consumers and embedder features. 

  The high-level plan here is to identify the common requirements from the embedder features and add corresponding helper methods (e.g. RenderFrameHost::CanUpdateTabLevelState) — then it will be much

  easier to reason about the impact of each consumer on each requirement.

  Please see this doc from sreejakshetty@ for more details.

  Going over the current places which check lifecycle states / RenderFrameHost::IsCurrent and thinking what the FF behaviour should be in each case is a good idea anyway :)

You received this message because you are subscribed to the Google Groups "chrome-fenced-frames" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chrome-fenced-fr...@google.com.
To view this discussion on the web visit https://groups.google.com/a/google.com/d/msgid/chrome-fenced-frames/CAJ_xCin9iucJ95790EGzBRFqtBRFagtwpGKcWBzwEm132ckLJg%40mail.gmail.com.

[Carlos Caballero]

On Wed, Apr 14, 2021 at 2:58 PM Alexander Timin <alt...@chromium.org> wrote:

Thanks Dom! 

Overall +1 to adding kFencedFrame both to RFH/RFHI LifecycleState and FrameTreeType from my side.

Some more details:

- Exclusivity: it's indeed an interesting question, especially w.r.t pages with fenced frames entering bfcache.

  My opinion here is that the LifecycleState is the abstraction which allows us to communicate to the //content embedders what a particular document should and should not do.

  Given that the restrictions for bfcache are strictly more strict than restrictions for fenced frames (as the document isn't visible to the user, is frozen and isn't allowed to do pretty much anything),

  so we should set the lifecycle state to such documents to kBackForwardCache (as the embedder shouldn't do anything differently for "fenced frame in bfcache" vs "iframe in bfcache" situations).

The more interesting case is a fenced frame in a prerender. But I think these will always be cross-origin right? So in that case we will not navigate them so we will not have the problem. 

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/navigation-dev/CALHg4nkLRFiZ08Y4twP7Cvr4PZyq%3D%3DnfLCcw%2BTDL-k0h651BYg%40mail.gmail.com.

[Alex Moshchuk]

On Wed, Apr 14, 2021 at 7:33 AM Carlos Caballero <carl...@chromium.org> wrote:

On Wed, Apr 14, 2021 at 2:58 PM Alexander Timin <alt...@chromium.org> wrote:

Thanks Dom! 

Overall +1 to adding kFencedFrame both to RFH/RFHI LifecycleState and FrameTreeType from my side.

Some more details:

- Exclusivity: it's indeed an interesting question, especially w.r.t pages with fenced frames entering bfcache.

  My opinion here is that the LifecycleState is the abstraction which allows us to communicate to the //content embedders what a particular document should and should not do.

  Given that the restrictions for bfcache are strictly more strict than restrictions for fenced frames (as the document isn't visible to the user, is frozen and isn't allowed to do pretty much anything),

  so we should set the lifecycle state to such documents to kBackForwardCache (as the embedder shouldn't do anything differently for "fenced frame in bfcache" vs "iframe in bfcache" situations).

The more interesting case is a fenced frame in a prerender. But I think these will always be cross-origin right? So in that case we will not navigate them so we will not have the problem. 

Adding a kFencedFrame FrameTreeType makes sense to me, but I'm less sure about a new LifecycleState.  I'm not fully up-to-speed on the fenced frame restrictions, so maybe my understanding is incorrect here, but in my mind fenced frames are much closer to kActive, compared to bfcached or prerendered documents which have many more restrictions.  And unlike bfcached or prerendered documents, which may become kActive again via activation, I don't think documents in fenced frames would ever be able to become kActive, if we loaded them as kFencedFrame to start with.  I also agree with the exclusivity concerns: fenced frame navigations would probably need to go through kPendingCommit and pending deletion states (so for example if we have to restrict what their unload scripts can/cannot do, we'd probably have to look at the FrameTreeType anyway).

To view this discussion on the web visit https://groups.google.com/a/google.com/d/msgid/chrome-fenced-frames/CAOL0AG9E9Xst4QsMMOKrVCBY51HrEPT0VnQ%3DC8Xcj2soGWj%2BXw%40mail.gmail.com.

[Alexander Timin]

Thinking a bit more about this, I now actually think that we should add a RenderFrameHost::Is(In)FencedFrame method rather than adding it as a LifecycleState (thanks to portals folks, who made some great points, more on that below).

In general, I think that the goal of the LifecycleState and similar APIs is to shield the //content embedders from the minute implementation details that they should not know about.

Sometimes it involves carefully selecting the signals to expose, sometimes — tweaking the implementation details (my favourite entry from legends seems relevant here), but overall the most important part is figure out which checks embedders

might want / should do and focus on those.

I've mentioned above that I'm not too worried about portals / fenced frames in the bfcache as "being in bfcache" is a more restrictive state than portal (e.g. there isn't anything that you would want to allow in bfcache, but disallow in portal) and the story is the same

with "pending deletion" — it's more strict than bfcache (and this allows us to transition from kInBackForwardCache state to kReadyToBeDeleted while evicting the page from bfcache).

Commit pending is a more interesting question, but I generally want to believe that it will go away sooner as a part of swap-at-commit project than many places will start depending on that.

However, the interaction between portals and fenced frames is the most interesting one here. My initial reaction was along the lines "portal should definitely be a lifecycle state, and a fenced frame is generally similar to a portal, so it should be a lifecycle state".

Now I believe that the conclusion here should be the opposite (thanks for all portals folks who pointed that out!) — given that portals and fenced frames are broadly similar, but the actual requirements are different, it's hard for them to be folded into a single flat enum due to FF-in-portal case.

Instead, adding an explicit method for "Is(In)FencedFrame" to RenderFrameHost(Impl) seems like the best way to go (as its value will never change, so the embedders won't have to worry about it changing) and then kPortal can be an actual lifecycle state.  

[Kentaro Hara]

I've mentioned above that I'm not too worried about portals / fenced frames in the bfcache as "being in bfcache" is a more restrictive state than portal (e.g. there isn't anything that you would want to allow in bfcache, but disallow in portal) and the story is the same
with "pending deletion" — it's more strict than bfcache (and this allows us to transition from kInBackForwardCache state to kReadyToBeDeleted while evicting the page from bfcache).

I'm actually a bit worried about this. "Being in BFcache" and "Being a fenced frame" are NOT exclusive. We could use the same state machine relying on the "strictness" but that's conceptually weird. Things conceptually weird have a risk of causing problems in the future. A state machine should be used only for exclusive states.

+1 to using RenderFrameHost::Is(In)FencedFrame.

--

Kentaro Hara, Tokyo