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

API Review: CoreWebView2Frame.FrameCreated #4982

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
215 changes: 215 additions & 0 deletions specs/NestedFrame.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
CoreWebView2Frame.FrameCreated API
===

# Background
At present, WebView2 enables developers to track only first-level
iframes, which are the direct child iframes of the main frame.
However, we see that WebView2 customers want to manage nested
iframes, such as recording the navigation history for a second
level iframe. To address this, we will introduce the
`CoreWebView2Frame.FrameCreated` API. This new API will allow
developers to subscribe to the nested iframe creation event,
giving them access to all properties, methods, and events of
[CoreWebView2Frame](https://learn.microsoft.com/dotnet/api/microsoft.web.webview2.core.corewebview2frame)
for the nested iframe.

To prevent unnecessary performance implication, WebView2 does
not track any nested iframes by default. It only tracks a nested
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to violate Windows Runtime guidelines, because it means that you don't get a CoreWebView2.FrameNavigationStarting unless you subscribe to the FrameCreated event (even if the event handler does nothing).

❌ DO NOT alter behavior based on whether an event handler is registered or the number of registered event handlers. In other words, registering an event handler that does nothing should have no effect on behavior.

An implementation is permitted to perform optimizations based on whether an event handler is registered. The most common case of this is bypassing an expensive operation if there is no way for the app to observe any of the operation’s side effects.

In this case, the app can observe the side effects (through the FrameNavigationStarting event).

We could have a new Boolean property on the frame ShouldRaiseNavigationEvents which defaults to true for first-level frames and false for deeper frames. (But see the componentization problem later.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to clarify the documentation here. The CoreWebView2.FrameNavigationStarting event will still be triggered for all child iframes, regardless of whether we subscribe to the CoreWebView2Frame.FrameCreated event or not. In this context, we are referring to the APIs in CoreWebView2Frame. We think that event handler matters because if developers do not subscribe to it, there is no need to expose any CoreWebView2Frame APIs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please clarify that this is an implementation note. This is only about an internal perf optimization - no observable behavior changes described here.

iframe if its parent iframe (`CoreWebView2Frame`) has subscribed
to the `CoreWebView2Frame.FrameCreated` API. For a page with
multi-level iframes, developers can choose to track only the
main page and first-level iframes (the default behavior), a
partial WebView2 frames tree with specific iframes of interest,
or the full WebView2 frames tree.

# Examples
### C++ Sample
```cpp
wil::com_ptr<ICoreWebView2> m_webview;
std::map<int, std::vector<std::wstring>> m_frame_navigation_urls;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::map<int, std::vector<std::wstring>> m_frame_navigation_urls;
std::map<UINT32, std::vector<std::wstring>> m_frame_navigation_urls;

Why not just use a UINT32 to match the type of FrameId?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix.

// In this example, a WebView2 application wants to manage the
// navigation of third-party content residing in second-level iframes
// (Main frame -> First-level frame -> Second-level third-party frames).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also a sample showing full depth frame tracking?

void TrackAllFrameNavigations()
{
   webView.CoreWebView2.FrameCreated += (_, args) =>
   {
      OnFrameCreated(args.Frame);
   };
}

void OnFrameCreated(CoreWebView2Frame frame)
{
    frame.FrameCreated += (_, e) => OnFrameCreated(e.Frame);
    frame.NavigationStarting += OnFrameNavigationStarting;
}

void OnFrameNavigationStarting(object sender, CoreWebView2NavigationStartingEventArgs e)
{
    // as before
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider adding an additional sample.

HRESULT RecordThirdPartyFrameNavigation() {
auto webview2_4 = m_webView.try_query<ICoreWebView2_4>();
// Track the first-level webview frame.
webview2_4->add_FrameCreated(
Callback<ICoreWebView2FrameCreatedEventHandler>(
[this](ICoreWebView2* sender, ICoreWebView2FrameCreatedEventArgs* args)
-> HRESULT {
// [AddFrameCreated]
wil::com_ptr<ICoreWebView2Frame> webviewFrame;
CHECK_FAILURE(args->get_Frame(&webviewFrame));
// Track nested (second-level) webview frame.
auto frame7 = webviewFrame.try_query<ICoreWebView2Frame7>();
frame7->add_FrameCreated(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a null check on frame7 in case the try_query fails.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix.

Callback<ICoreWebView2FrameChildFrameCreatedEventHandler>(
[this](
ICoreWebView2Frame* sender,
ICoreWebView2FrameCreatedEventArgs* args) -> HRESULT
{
wil::com_ptr<ICoreWebView2Frame> webviewFrame;
CHECK_FAILURE(args->get_Frame(&webviewFrame));
wil::com_ptr<ICoreWebView2Frame2> frame2 =
webviewFrame.try_query<ICoreWebView2Frame2>();
if (frame2)
{
// Subscribe to nested (second-level) webview frame navigation
// starting event.
frame2->add_NavigationStarting(
Callback<ICoreWebView2FrameNavigationStartingEventHandler>(
[this](
ICoreWebView2Frame* sender,
ICoreWebView2NavigationStartingEventArgs* args) -> HRESULT {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ICoreWebView2NavigationStartingEventArgs* args) -> HRESULT {
ICoreWebView2NavigationStartingEventArgs* args) noexcept -> HRESULT {

Fail fast if an exception comes out of the [] or push_back.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix.

// Manage the navigation, e.g. cancel the
// navigation if it's on block list.
UINT32 frameId = 0;
auto frame5 = wil::com_ptr<ICoreWebView2Frame>(sender)
.try_query<ICoreWebView2Frame5>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing null check from try_query. Maybe we should use ICoreWebView2Frame5 in the outer lambda, so that we don't even get here if v5 is not supported. Then that would allow us to use query() instead of try_query().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider simple fix of null check for simple sample code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be simplified to

auto frame5 = wil::try_com_query<ICoreWebView2Frame5>(sender);

avoids an unnecessary temporary com_ptr.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix.

CHECK_FAILURE(frame5->get_FrameId(&frameId));
wil::unique_cotaskmem_string uri;
CHECK_FAILURE(args->get_Uri(&uri));
// Log the navigation history per frame Id.
m_frame_navigation_urls[(int)frameId].push_back(uri.get());
return S_OK;
})
.Get(),
nullptr);
}
return S_OK;
})
.Get(),
nullptr);
// [AddFrameCreated]
return S_OK;
})
.Get(),
nullptr);
}
```
### C# Sample
```c#
var _frameNavigationUrls = new Dictionary<UINT32, List<string>>();
// In this example, a WebView2 application wants to manage the
// navigation of third-party content residing in second-level iframes
// (Main frame -> First-level frame -> second-level third-party frames).
void RecordThirdPartyFrameNavigation() {
webView.CoreWebView2.FrameCreated += (sender, args) =>
{
// Track nested (second-level) webview frame.
args.Frame.FrameCreated += (frameCreatedSender, frameCreatedArgs) =>
{
CoreWebView2Frame childFrame = frameCreatedArgs.Frame;
childFrame.NavigationStarting += HandleChildFrameNavigationStarting;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}
};
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix.

}

void HandleChildFrameNavigationStarting(object sender,
CoreWebView2NavigationStartingEventArgs args)
{
// Manage the navigation, e.g. cancel the navigation
// if it's on block list.
CoreWebView2Frame frame = (CoreWebView2Frame)sender;
if (!_frameNavigationUrls.ContainsKey(frame.FrameId))
{
_frameNavigationUrls[frame.FrameId] = new List<string>();
}
// Log the navigation history per frame Id.
_frameNavigationUrls[frame.FrameId].Add(args.Uri);
}
```

# API Details
## C++
```C++
/// Receives `FrameCreated` events.
interface ICoreWebView2FrameChildFrameCreatedEventHandler : IUnknown {
/// Provides the event args for the corresponding event.
HRESULT Invoke(
[in] ICoreWebView2Frame* sender,
[in] ICoreWebView2FrameCreatedEventArgs* args);
}

/// This is the ICoreWebView2Frame interface.
interface ICoreWebView2Frame7 : IUnknown {
/// Adds an event handler for the `FrameCreated` event.
/// Raised when a new direct descendant iframe is created.
/// Handle this event to get access to `ICoreWebView2Frame` objects.
/// Use `ICoreWebView2Frame::add_Destroyed` to listen for when this
/// iframe goes away.
///
/// \snippet ScenarioWebViewEventMonitor.cpp AddFrameCreated
HRESULT add_FrameCreated(
[in] ICoreWebView2FrameChildFrameCreatedEventHandler* eventHandler,
[out] EventRegistrationToken* token);

/// Removes an event handler previously added with `add_FrameCreated`.
HRESULT remove_FrameCreated(
[in] EventRegistrationToken token);
}
```

## C#
```c#
namespace Microsoft.Web.WebView2.Core
{
runtimeclass CoreWebView2Frame
{
[interface_name("Microsoft.Web.WebView2.Core.ICoreWebView2Frame7")]
{
event Windows.Foundation.TypedEventHandler<CoreWebView2Frame, CoreWebView2FrameCreatedEventArgs> FrameCreated;
}
}
}
```

# Appendix
## Impacted API
### `CoreWebView2Frame.PermissionRequested` and `CoreWebView2Frame.ScreenCaptureStarting`
In the current case of nested iframes, the [PermissionRequested](https://learn.microsoft.com/microsoft-edge/webview2/reference/winrt/microsoft_web_webview2_core/corewebview2frame#permissionrequested)
and [ScreenCaptureStarting](https://learn.microsoft.com/microsoft-edge/webview2/reference/winrt/microsoft_web_webview2_core/corewebview2frame#screencapturestarting)
events will be raised from the top-level iframe. With the support
of tracking nested iframes, we can now handle these requests directly
within the nested iframe. Specifically, these requests are raised to
the nearest tracked frame, which is the `CoreWebView2Frame` closest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could lead to a problem if an app combines multiple components, and one of them adds a FrameCreated handler and accidentally messes up the other component which expected the event from the top-level frame. Is the answer "Yeah, you can't really componentize your app like that because features used by one app might have adverse consequences far away." So it means I can't have a "app usage telemetry" component that hooks all frame activity and logs it, because that would accidentally break my "permission manager" component.

From the Windows Runtime design guidelines:

A Well-Designed API Avoids Shared Behavior

A developer is often unpleasantly surprised when he or she uses one API that causes a side-effect elsewhere.

☑ DO design APIs to allow multiple components running in the same process to use the API without having to explicitly coordinate, for example, two plug-ins loaded into the same instance of Internet Explorer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For PermissionRequested API, there are two factors. One is the presence of the FrameCreated handler, and the other is the need to handle the PermissionRequested API. Simply subscribing to the FrameCreated handler does not affect where the PermissionRequested event is actually handled.
Suppose there is a permission request from frame D. Developers want to handle the PermissionRequested event from the top-level iframe B in component I. Meanwhile, in component II, developers have an "app usage telemetry" that hooks all frame activity. Unless the developers subscribe to the permission request and Handled in either frame D or frame C, otherwise the PermissionRequested event will still bubble up to the top-level iframe and handle in component I.

// Example:
//    A (main frame/CoreWebView2)
//    |
//    B (first-level iframe/CoreWebView2Frame)
//    |  
//    C (nested iframe)
//    |
//    D (nested iframe)

But I think this kind of shared behavior should be avoid by developers. Even if we, align with the browser, track all iframes by default and the PermissionRequested event starts to fire on the iframe where it originates, there could be shared behavior issue. If developers handle the PermissionRequested event in a nested iframe within one component, then another component won't have the opportunity to handle the PermissionRequested event at the top-level iframe.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issue after discussion: event bubbles out in existing event bubbling pattern.

to the frame that initiates the request (from bottom to top).
```
// Example:
// A (main frame/CoreWebView2)
// |
// B (first-level iframe/CoreWebView2Frame)
// |
// C (nested iframe)
// |
// D (nested iframe)
```
Suppose there's a `PermissionRequest` comes from D.
* If D is a tracked frame (`CoreWebView2Frame`), then D is the
closet tracked frame from which the request will be raised from.
* If D is not being tracked, and C is a tracked frame. Then C
is the closet tracked frame from which the request will be
raised from.
* If neither C nor D is tracked, then B is the closet tracked
frame from which the request will be raised. This case applies
to current `PermissionRequested` developers, as they haven't
subscribe to the `CoreWebView2Frame.FrameCreated` event.
Therefore, requests originating from iframes will still be
raised from the first-level iframe.

If the `PermissionRequested` event is not handled in the current
tracked frame, the request will propagate to its parent
`CoreWebView2Frame`, or to `CoreWebView2` if the parent frame
is the main frame. For example, if frame D is tracked but does
not handle the request, the request will bubble up to frame C.
If frame C handles the request, it will not propagate further
to its parent frame B.

### `CoreWebView2.ProcessFailed`
With the support of tracking nested iframes, the processes
which support these nested iframes will be also tracked by
[ProcessFailed](https://learn.microsoft.com/dotnet/api/microsoft.web.webview2.core.corewebview2.processfailed).
As we only track processes running tracked iframes, existing
developers will not receive any process failed events specific
to nested iframes as they haven't subscribe to the
`CoreWebView2Frame.FrameCreated` event.