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

Add Spec: Critical Restart Required #4657

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dianaqu
Copy link
Contributor

@dianaqu dianaqu commented Jul 1, 2024

An Critical Restart Required API to notify user for restart if needed

* add critical api

---------

Co-authored-by: Diana Qu <xiaqu@microsoft.com>
Co-authored-by: David Risney <dave@deletethis.net>
@dianaqu dianaqu requested a review from david-risney July 1, 2024 21:52
}
}

void WebView_CriticalRestartRequired(object sender, object e)
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
void WebView_CriticalRestartRequired(object sender, object e)
void WebView_CriticalRestartRequired(CoreWebView2Environment sender, object e)

## Win32 C++
```cpp
void CoreWebView2InitializationCompleted() {
wil::com_ptr<ICoreWebView2Environment> m_webViewEnvironment;
Copy link
Contributor

Choose a reason for hiding this comment

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

Declare this as a member variable rather than a local, and then the reader realizes that it's initialized somewhere out-of-scope to this example

used to perform A/B testing or remotely disable features) configurations to toggle feature flags.
However, once these payloads are received, there is no way to restart the WebView2 and apply the
payload. The purpose of this API is to detect such critical payloads and inform the end developer
so they may restart their app or their WebView2 or other appropriate action.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does "restart" mean e.g. AppInstance.Restart? Would help to show this in the example.

Is there a way to just restart the WV2 though?

///
/// For apps with multiple processes that host WebView2s that share the same user data folder you
/// need to make sure all WebView2 instances are closed and the associated WebView2 Runtime
/// browser process exits. See `BrowserProcessExited` for more details.
Copy link
Contributor

Choose a reason for hiding this comment

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

If there's another app using WV2, but not the same UDF, then it won't cause a problem?

/// `CriticalRestartRequired` gives you the awareness of these necessary WebView2 restarts,
/// allowing you to resolve issues faster than waiting for your end users to restart the app.
/// Depending on your app you may want to prompt your end users in some way to give
/// them a chance to save their state before you restart the WebView2.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you, this is a great customer-focused API description :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants