Integrating Fenced Frames with MPArch long-tail feature development

215 views
Skip to first unread message

Dominic Farolino

unread,
Apr 13, 2021, 5:57:58 PM4/13/21
to navigation-dev, Prerender, chrome-fen...@google.com
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

unread,
Apr 13, 2021, 9:30:32 PM4/13/21
to Dominic Farolino, navigation-dev, Prerender, chrome-fenced-frames
  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

    unread,
    Apr 13, 2021, 9:35:50 PM4/13/21
    to Kentaro Hara, navigation-dev, Prerender, chrome-fenced-frames
    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

    unread,
    Apr 13, 2021, 10:04:14 PM4/13/21
    to Dominic Farolino, navigation-dev, Prerender, chrome-fenced-frames
    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

    unread,
    Apr 13, 2021, 11:39:48 PM4/13/21
    to Kentaro Hara, Matt Falkenhagen, navigation-dev, Prerender, chrome-fenced-frames
    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

    Matt Falkenhagen

    unread,
    Apr 14, 2021, 5:28:44 AM4/14/21
    to Dominic Farolino, Kentaro Hara, navigation-dev, chrome-fenced-frames
    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.
    Reply all
    Reply to author
    Forward
    0 new messages