• Fabio Rocha's avatar
    Reland "desktop-pwas: Introduce shared file/protocol handler registration logic" · 65390511
    Fabio Rocha authored
    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: default avatarGreg Thompson <grt@chromium.org>
    Reviewed-by: default avatarScott Violet <sky@chromium.org>
    Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
    Reviewed-by: default avatarDavid Bienvenu <davidbienvenu@chromium.org>
    Commit-Queue: Fabio Rocha <fabio.rocha@microsoft.com>
    Cr-Commit-Position: refs/heads/master@{#818684}
    65390511
BUILD.gn 7.16 KB