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 for StartInBackgroundMode.md #2781

Merged
merged 10 commits into from
Sep 30, 2022

Conversation

JosephJin0815
Copy link

This is the review for the new StartInBackgroundMode API.

@JosephJin0815 JosephJin0815 added the API Proposal Review WebView2 API Proposal for review. label Sep 9, 2022
@JosephJin0815 JosephJin0815 changed the title Create StartInBackgroundMode.md Add spec for StartInBackgroundMode.md Sep 9, 2022
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
specs/StartInBackgroundMode.md Outdated Show resolved Hide resolved
Copy link
Contributor

@david-risney david-risney left a comment

Choose a reason for hiding this comment

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

Looking good. Please fix these small issues and then complete this PR, open a new PR to main, and let me know and I will schedule an API review. Thanks.

/// Set `CreationPriority` to `COREWEBVIEW2_CREATION_PRIORITY_HIGH` will attempt to
/// create the WebView2 browser process with high priority.
/// Default is `COREWEBVIEW2_CREATION_PRIORITY_HIGH`.
/// Note that 1)The host app's priority must be at least normal for
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
/// Note that 1)The host app's priority must be at least normal for
/// Note that the host app's priority must be at least normal for

/// create the WebView2 browser process with high priority.
/// Default is `COREWEBVIEW2_CREATION_PRIORITY_HIGH`.
/// Note that 1)The host app's priority must be at least normal for
/// `COREWEBVIEW2_CREATION_PRIORITY_HIGH` to be applied. Else, the WebView2 browser process
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
/// `COREWEBVIEW2_CREATION_PRIORITY_HIGH` to be applied. Else, the WebView2 browser process
/// `COREWEBVIEW2_CREATION_PRIORITY_HIGH` to be applied. Otherwise the WebView2 browser process

/// Note that 1)The host app's priority must be at least normal for
/// `COREWEBVIEW2_CREATION_PRIORITY_HIGH` to be applied. Else, the WebView2 browser process
/// still get created with normal priority even with `COREWEBVIEW2_CREATION_PRIORITY_HIGH` setting.
/// 2)Currently `CreationPriority` only applies to the creation of the WebView2 browser process.
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
/// 2)Currently `CreationPriority` only applies to the creation of the WebView2 browser process.
/// Additionally, currently `CreationPriority` only applies to the creation of the WebView2 browser process.

@JosephJin0815 JosephJin0815 merged commit b84c803 into StartInBackgroundMode Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Proposal Review WebView2 API Proposal for review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants