Reland "desktop-pwas: Introduce shared file/protocol handler registration logic"
This is a reland of https://chromium-review.googlesource.com/c/chromium/src/+/2462383 I needed to update Win7 tests to account for particularities of that OS, namely the fact that the extension is omitted for launcher filenames since those are also used as the app's display name. --- Reason for revert: The tests added here (WebAppHandlerRegistrationUtilsWinTest.*) are failing on multiple Win 7 bots, see https://bugs.chromium.org/p/chromium/issues/detail?id=1139503 Original change's description: > desktop-pwas: Introduce shared file/protocol handler registration logic > > This CL refactors Windows file handler registration in a way that > exposes shared logic useful for protocol handler registration. Protocol > handler registration that consumes the shared logic will be added in a > subsequent CL. This design is detailed in section 4.4 of [1] with a > summary given below. > > The refactor splits up registration by functional concerns, > with pieces that may be shared with protocol handler registration living > in web_app_handler_registration_utils_win.h/.cc. > web_app_file_handler_registration_win.h/.cc is now significantly smaller > and addresses file-handler-specific concerns in addition to consuming > the utils API. > > File handler registration consists of several pieces: > > 1) Create an app registry entry: HKCU\Software\Classes\<app_progid> > 2) Copy (or hardlink) chrome_pwa_launcher.exe from browser install > directory to <profile_dir>\web_applications\<app_id>\<app_name.exe> > 3) Reregister an app (steps 1 and 2) with a profile-specific name when > the same app is installed to a different profile > (e.g. "App" -> "App (Profile 1)") > 4) Create a file type association registry entry: > HKCU\Software\Classes\.<file_ext> > > In this approach, 1 & 4 are contained in the existing > ShellUtil::AddFileAssociations function. 2 & 3 are moved to utils > (CreateAppLauncherFile and CheckAndUpdateExternalInstallations). > > Important note about this design: > - Reregistration logic (3) is altered in this approach. Currently, > pieces 1, 2, 4 are all executed during reregistration. Because (4) is a > file-handler only concern, it wouldn't make sense to use existing logic > from the protocol registration flow. Reregistering (1) and (2) are > sufficient to rename a duplicate app - this is because file handler > registry entries (4) only reference the progid of an app which doesn't > change if its display name is updated (or if the update fails). > CheckAndUpdateExternalInstallations only updates necessary registration > pieces to reflect an updated name while leaving existing file/protocol > pieces in place. > > Alternative design: > crrev.com/c/2309759 previously attempted this refactor by splitting the > registration process into a shared app-level API instead of a Utils API. > As a result, ShellUtil functions that are currently atomic > (AddFileAssociations) were split into non-atomic app-level and file > handler pieces. The design was abandoned for this reason. > > [1] https://docs.google.com/document/d/1NHlWLjAPZ-dyxcz3AoTWibeerDeHW7Vqrx6FmhB0XmE/edit#heading=h.qsh70q > > Bug: 1019239 > Change-Id: I5f727a5ca1483efd21935c0201ee54474a94e598 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462383 > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Daniel Murphy <dmurph@chromium.org> > Reviewed-by: David Bienvenu <davidbienvenu@chromium.org> > Reviewed-by: Greg Thompson <grt@chromium.org> > Commit-Queue: Fabio Rocha <fabio.rocha@microsoft.com> > Cr-Commit-Position: refs/heads/master@{#818065} Bug: 1019239 Change-Id: I06cfeb90e2e3c7b438210250fc95917e4362e425 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2482185Reviewed-by:Greg Thompson <grt@chromium.org> Reviewed-by:
Scott Violet <sky@chromium.org> Reviewed-by:
Daniel Murphy <dmurph@chromium.org> Reviewed-by:
David Bienvenu <davidbienvenu@chromium.org> Commit-Queue: Fabio Rocha <fabio.rocha@microsoft.com> Cr-Commit-Position: refs/heads/master@{#818684}
Showing
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
Please register or sign in to comment