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

Jump to bottom

refactor: move PDF viewer to OOPIF #41728

Merged
merged 1 commit into from Apr 9, 2024
Merged

refactor: move PDF viewer to OOPIF #41728

merged 1 commit into from Apr 9, 2024

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Mar 28, 2024 •
edited

Description of Change

Refs https://issues.chromium.org/issues/40268279
Closes #41726.

Incorporates:

Refactors our PDF viewer implementation to work with upstream migration from GuestView to OOPIF.

Checklist

Release Notes

Notes: none

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Mar 28, 2024
codebytere
codebytere commented Mar 28, 2024
View reviewed changes
shell/browser/extensions/api/management/electron_management_api_delegate.cc
@@ -154,7 +154,7 @@ void ElectronManagementAPIDelegate::InstallOrLaunchReplacementWebApp(

void ElectronManagementAPIDelegate::EnableExtension(
content::BrowserContext* context,
const std::string& extension_id) const {
Copy link
Member Author

Choose a reason for hiding this comment

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

not strictly related - drive by change from CL:5268445

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Apr 4, 2024
@codebytere codebytere marked this pull request as ready for review April 4, 2024 20:48
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Apr 4, 2024
@codebytere codebytere added no-backport semver/patch backwards-compatible bug fixes labels Apr 4, 2024
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Apr 5, 2024
@deepak1556
Copy link
Member

High level question, is there plan to run the pdf tests with the feature flag enabled ?

@codebytere
Copy link
Member Author

codebytere commented Apr 8, 2024 •
edited

@deepak1556 I ran them when I added this but as a rule I don't think so - mostly be we historically haven't really tested with flipped base::Feature flags we don't want to let end-users disable if they're going to be flipped upstream in the short term

@jkleinsc jkleinsc merged commit 38ef9a7 into main Apr 9, 2024
15 checks passed
@jkleinsc jkleinsc deleted the pdf-stream branch April 9, 2024 23:59
@release-clerk Release Clerk
Copy link

release-clerk bot commented Apr 9, 2024

No Release Notes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkleinsc jkleinsc jkleinsc approved these changes

@deepak1556 deepak1556 deepak1556 approved these changes

@nornagon nornagon Awaiting requested review from nornagon

@MarshallOfSound MarshallOfSound Awaiting requested review from MarshallOfSound

Assignees
No one assigned
Labels
no-backport semver/patch backwards-compatible bug fixes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PDF Viewer moving to OOPIF upstream
3 participants
@codebytere @deepak1556 @jkleinsc