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.

Sign up for GitHub

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

Multiprofile's Delete API modify #3069

Open
wants to merge 8 commits into
base: MultipleFile_Delete
Choose a base branch
from
Open

Multiprofile's Delete API modify #3069

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3b46d95
Multiprofile's Delete API modify
Dec 15, 2022
710afd2
add add_ProfileDeleted/remove_ProfileDeleted SPEC
Dec 19, 2022
3a0c4aa
comments fix
Jan 4, 2023
e834d1e
remove staging and use the expected public name
Jan 5, 2023
70e7a7f
fix comments
Jan 5, 2023
4e428e2
comments fix
Jan 16, 2023
33cda74
add spec of 'IsClosed' and Enum value of 'Closed' event
Feb 3, 2023
767d420
remove other enum value and add them in latter PR
Feb 7, 2023
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
137 changes: 115 additions & 22 deletions specs/MultiProfile.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,26 +178,63 @@ HRESULT AppWindow::DeleteProfile(ICoreWebView2* webView2)

void AppWindow::RegisterEventHandlers()
{
CHECK_FAILURE(m_webView->add_Closed(
BOOL isWebview2Closed = FALSE;
CHECK_FAILURE(webView2->get_IsClosed(&isWebview2Closed));
if (!isWebview2Closed) {
CHECK_FAILURE(m_webView->add_Closed(
Microsoft::WRL::Callback<ICoreWebView2ClosedEventHandler>(
[this](ICoreWebView2_20* sender, ICoreWebView2EventArgs* args)
{
COREWEBVIEW2_CLOSED_REASON reason;
CHECK_FAILURE(args->get_Reason(&reason));
if (reason == COREWEBVIEW2_CLOSED_REASON::COREWEBVIEW2_CLOSED_REASON_PROFILE_DELETED)
std::wstring message;
switch (reason)
{
RunAsync( [this]()
case COREWEBVIEW2_CLOSED_REASON::COREWEBVIEW2_CLOSED_REASON_SHUTDOWN:
{
std::wstring message =
L"The webview2 has been closed and the reason is profile "
L"has been marked as deleted.";
MessageBox(
m_mainWindow, message.c_str(), L"webview2 closed", MB_OK);
CloseAppWindow();
});
message = L"The CoreWebView2 has closed because the corresponding "
L"CoreWebView2Controller had its Close method called either "
L"explicitly or implicitly.";
break;
}
case COREWEBVIEW2_CLOSED_REASON::
COREWEBVIEW2_CLOSED_REASON_BROWSER_PROCESS_FAILURE:
{
message = L"The CoreWebView2 has closed because its browser process "
L"crashed. The Closed event will be raised after the "
L"ProcessFailed event is raised.";
break;
}
case COREWEBVIEW2_CLOSED_REASON::
COREWEBVIEW2_CLOSED_REASON_BROWSER_PROCESS_EXITED:
{
message = L"The CoreWebView2 has closed because its browser process has "
L"exited.";
break;
}
case COREWEBVIEW2_CLOSED_REASON::
COREWEBVIEW2_CLOSED_REASON_PARENT_WINDOW_CLOSED:
{
message = L"The CoreWebView2 has closed because its parent window was "
L"closed.";
break;
}
case COREWEBVIEW2_CLOSED_REASON::COREWEBVIEW2_CLOSED_REASON_PROFILE_DELETED:
{
message = L"The CoreWebView2 has closed because its corresponding "
L"Profile has been or marked as deleted.";
break;
}
}
RunAsync([this, message]()
{
MessageBox(
m_mainWindow, message.c_str(), L"webview2 closed", MB_OK);
CloseAppWindow();
});
return S_OK;
}).Get(), nullptr));
}
}
```

Expand Down Expand Up @@ -274,20 +311,48 @@ public DeleteProfile(CoreWebView2Controller controller)

void WebView_CoreWebView2InitializationCompleted(object sender, CoreWebView2InitializationCompletedEventArgs e)
{
webView.CoreWebView2.Closed += CoreWebView2_Closed;
if (!webView.IsClosed) {
webView.CoreWebView2.Closed += CoreWebView2_Closed;
}
}

private void CoreWebView2_Closed(object sender, CoreWebView2ClosedEventArgs e)
{
if (e.Reason == CoreWebView2ClosedReason.ProfileDeleted)
String message;
switch (e.Reason)
{
this.Dispatcher.InvokeAsync(() =>
case CoreWebView2ClosedReason.ShutDown:
{
message = "The CoreWebView2 has closed because the corresponding CoreWebView2Controller had its Close method called either explicitly or implicitly."
break;
}
case CoreWebView2ClosedReason.BrowserProcessFailure:
{
message = "The CoreWebView2 has closed because its browser process crashed. The Closed event will be raised after the ProcessFailed event is raised."
break;
}
case CoreWebView2ClosedReason.BrowserProcessExited:
{
message = "The CoreWebView2 has closed because its browser process has exited."
break;
}
case CoreWebView2ClosedReason.ParentWindowClosed:
{
message = "The CoreWebView2 has closed because its parent window was closed."
break;
}
case CoreWebView2ClosedReason.ProfileDeleted:
{
String message = "The webview2 has been closed and the reason is profile has been marked as deleted";
MessageBox.Show(message);
Close();
});
message = "The CoreWebView2 has closed because its corresponding Profile has been or marked as deleted."
break;
}
}

this.Dispatcher.InvokeAsync(() =>
{
MessageBox.Show(message);
Close();
});
}
```

Expand Down Expand Up @@ -427,14 +492,33 @@ interface ICoreWebView2_9 : IUnknown {
/// Remove an event handler previously added with `add_Closed`.
HRESULT remove_Closed(
[in] EventRegistrationToken token);

/// `TRUE` if WebView2 has moved to the Closed state. Use `add_Closed` API
/// to get the specific closed reason. For more information see `add_Closed`.
[propget] HRESULT IsClosed([out, retval] BOOL* value);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

In addition we should add a property where you can check if the CoreWebView2 is closed or not. This way end dev's asynchronous code that may be in progress when the profile is deleted can check if the CoreWebView2 has closed and take appropriate action.

/// Returns TRUE if the CoreWebView2 is closed. Once closed, CoreWebView2 
/// events will no longer be raised and methods will immediately return failure
/// unless otherwise noted in the API documentation. This property will continue
/// to work after the CoreWebView2 is closed.
[propget] HRESULT IsClosed([out] BOOL* value);

Copy link
Contributor Author

@yunate yunate Jan 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think do not need this function. Because, close webview and Closed event handle function called are in a same therad and a same loop. So, if the Closed event handle function called, the user has known the webview has been closed, they can record a flag in this function; if before Closed event handle function called, the webview is always still alive.


/// The reason of webview2 closed.
[v1_enum]
typedef enum COREWEBVIEW2_CLOSED_REASON {
/// Indicates that the reason of webview2 closed is its corresponding
/// Profile has been or marked as deleted.
COREWEBVIEW2_CLOSED_REASON_PROFILE_DELETED,
/// The CoreWebView2 has closed because the corresponding
/// CoreWebView2Controller had its Close method called either explicitly or
/// implicitly.
COREWEBVIEW2_CLOSED_REASON_SHUTDOWN,
Copy link
Contributor

Choose a reason for hiding this comment

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

If developers explicitly called Close() to close the webview, wondering whether we need to raise the Closed event.

And, what scenario do you imagine that the Close() method is called implicitly? Is it already covered by other enums?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove this Close event's Enum value and add them in latter PR.


/// The CoreWebView2 has closed because its browser process crashed.
/// The Closed event will be raised after the ProcessFailed event is raised.
COREWEBVIEW2_CLOSED_REASON_BROWSER_PROCESS_FAILURE,

/// The CoreWebView2 has closed because its browser process has exited.
COREWEBVIEW2_CLOSED_REASON_BROWSER_PROCESS_EXITED,
yunate marked this conversation as resolved.
Show resolved Hide resolved

/// The CoreWebView2 has closed because its parent window was closed.
COREWEBVIEW2_CLOSED_REASON_PARENT_WINDOW_CLOSED,

/// The CoreWebView2 has closed because its corresponding Profile has been or
/// marked as deleted.
COREWEBVIEW2_CLOSED_REASON_PROFILE_DELETED
} COREWEBVIEW2_CLOSED_REASON;
Copy link
Contributor

Choose a reason for hiding this comment

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

The two other cases we should add here (we can get the urgent work's implementation in first, and the less urgent parts implementation in a later PR) are for normal webview2 shutdown, and for browser process crash. Maybe this:

/// The CoreWebView2 has closed because the corresponding
/// CoreWebView2Controller had its Close method called either
/// explicitly or implicitly.
COREWEBVIEW2_CLOSED_REASON_SHUTDOWN,

/// The CoreWebView2 has closed because its browser process
/// crashed. The Closed event will be raised after the ProcessFailed
/// event is raised.
COREWEBVIEW2_CLOSED_REASON_BROWSER_PROCESS_FAILURE,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will create a work item to trace this things.

Copy link
Contributor

Choose a reason for hiding this comment

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

Additional entries for this:

/// The CoreWebView2 has closed because the corresponding
/// CoreWebView2Controller had its Close method called either
/// explicitly or implicitly.
COREWEBVIEW2_CLOSED_REASON_SHUTDOWN,

/// The CoreWebView2 has closed because its browser process
/// crashed. The Closed event will be raised after the ProcessFailed
/// event is raised.
COREWEBVIEW2_CLOSED_REASON_BROWSER_PROCESS_FAILURE,

/// The CoreWebView2 has closed because its browser process
/// has exited. 
COREWEBVIEW2_CLOSED_REASON_BROWSER_PROCESS_EXITED,

/// The CoreWebView2 has closed because its parent window
/// was closed.
COREWEBVIEW2_CLOSED_REASON_PARENT_WINDOW_CLOSED

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do I need to add those to the API SPEC, or add them one by one in a later PR?


/// Receives the webview2 `Closed` event.
Expand Down Expand Up @@ -489,7 +573,12 @@ namespace Microsoft.Web.WebView2.Core
CoreWebView2ControllerWindowReference ParentWindow,
CoreWebView2ControllerOptions options);
}


runtimeclass WebView2
{
bool IsClosed { get; };
}

runtimeclass CoreWebView2
{
// ...
Expand All @@ -503,7 +592,11 @@ namespace Microsoft.Web.WebView2.Core

enum CoreWebView2ClosedReason
{
ProfileDeleted = 0,
ShutDown,
BrowserProcessFailure,
BrowserProcessExited,
ParentWindowClosed,
ProfileDeleted,
};

runtimeclass CoreWebView2ClosedEventArgs
Expand Down