Skip to content

feat(bundler): extend webview2 installation options, closes #2882 #2452#4466

Merged
lucasfernog merged 14 commits intodevfrom
feat/webview2-install-options
Jun 26, 2022
Merged

feat(bundler): extend webview2 installation options, closes #2882 #2452#4466
lucasfernog merged 14 commits intodevfrom
feat/webview2-install-options

Conversation

@lucasfernog
Copy link
Member

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

This PR enhances the WiX installer options for webview2 installation:

  • included option to embed the webview2 bootstrapper - increases bundle by 1.8~ MB
  • included option to embed the webview2 standalone (offline) installer - increases bundle by 127~ MB
  • current skip_install and webview_fixed_runtime_path options are no longer needed but kept for backwards compatibility

The default webview2 install option is now to embed the webview2 bootstrapper to fix installers on Windows 7.

@lucasfernog lucasfernog requested a review from a team as a code owner June 25, 2022 01:02
@lucasfernog
Copy link
Member Author

@FabianLars can you review this one?

@lucasfernog
Copy link
Member Author

I tested the installing of the new modes but i'm having issues running the apps in my VM :( probably too old The procedure entry point EventSetInformation could not be located in the dynamic link library advapi32.dll.

@FabianLars
Copy link
Member

FabianLars commented Jun 25, 2022

Okay i don't have much time today so i only report what happens and won't look into why it happens.

  1. the fixedRuntime variant doesn't work. It falls back to skip.

  2. All my deps are on the latest version (well, i have to use this branch after all), but i still have that "File in Use" prompt:
    grafik
    A pretty huge list, but half of it are Firefox processes.
    And i only saw this with the skip option, but this may be a coincidence since it was the first one i tried.

  3. How do we ship this without breaking anything lol. Pretty unlucky that users have to update tauri, tauri-build and the cli to make this work, but what's actually is a problem is that only updating the cli without the crates, it will fail to build.

  4. Do we really want to default to embedBootstrapper? i mean sure, it's needed for win7 and iirc 8 too, but idk

  5. unlucky sideeffect is that the updater artifacts see the same size increases (ofc they do, it's the same file lol). Since the app doesn't even start without it downloadBootstrapper should be fine most of the time. That said this is imo pretty much a non-issue if we'd default to downloadBootstrapper instead.
    The advantage of this is that offlineInstaller users will update the webview alongside the app.

  6. What do you think about integrating [bug] The standard Windows installation has bad UX for installing WebView2 as a dependency #4389 into this PR for the downloadBootstrapper variant (still using the powershell based variant)? If not i'm fine with keeping embedBootstrapper as the default because it really is way nicer :D

  7. Should we install webview2 silently instead? (also talked about in [bug] The standard Windows installation has bad UX for installing WebView2 as a dependency #4389)

  8. Apart from fixedRuntime mentioned in no1, all modes worked fine in Windows 10, gonna update this comment with the results of win7 in a few.
    Edit: Windows 7 result:
    - downloadBootstrapper doesn't work as expected.
    - embedBootstrapper and offlineInstaller work beautifully! A weird difference between these modes is that with the offlineInstaller you need to manually click on "close" (on the wv2 installer) so that the app installer can continue (win7 and win10).

@lucasfernog
Copy link
Member Author

  1. I didn't test this one yet, will fix in a bit.
  2. Can you check the logs to see which file is causing it? Shouldn't happen.
  3. This has been a problem since day 1 :/ I'm not sure we can fix it with the way the CLI works.
  4. I'm not sure, I'm fine with defaulting to downloadBootstrapper. We can document this instead in the distributing guide.
  5. I think I'll need to change the updater mode :D
  6. and 7. I'll check that.

@lucasfernog
Copy link
Member Author

Don't worry about testing it today, it can wait till next week. Also don't investigate too much the why, I just wanted some help double checking it. Will keep working on it later.

@FabianLars
Copy link
Member

I updated the comment with win7 results.

And here's the log: msi.log

Info 1603.The file C:\WINDOWS\system32\msvcp140.dll is being held in use. Close that application and retry.

@lucasfernog
Copy link
Member Author

Hey we don't use msvcp140.dll anymore 😂 is it in the target/release folder somehow? Can you see if it's referenced in target/release/wix/x64/main.wxs?

@FabianLars
Copy link
Member

Yes to both. no idea how tho, it was a clean build 🤔 idk maybe windows is trolling me again, sorry for the noise ✌️

@lucasfernog lucasfernog force-pushed the feat/webview2-install-options branch from bb0b4ad to 3661ba9 Compare June 25, 2022 17:05
@lucasfernog
Copy link
Member Author

Changed the updater MSI to use DownloadBootstrapper if OfflineInstaller or EmbedBootstrapper is set.

@lucasfernog
Copy link
Member Author

Maybe one of your dependencies is copying the dll to the target folder @FabianLars let me know if you still hit this issue and maybe share a cargo.toml/cargo.lock to use.

@FabianLars
Copy link
Member

I think my windows is broken (again). if I manually move the target folder into the trash (which i did) it somehow ends up in the new target folder again.
If I empty the trash or use cargo clean instead everything works fine...

P.S. and yes this is somewhat reproducible, but it only happens on my main machine so I guess it's time for the monthly windows reinstallation 😂

@lucasfernog
Copy link
Member Author

Ah ok. I'll add an option for silent installation, but I don't know what else I can do to resolve #4389

@FabianLars
Copy link
Member

but I don't know what else I can do to resolve

Adapted from #2452 (comment)

<CustomAction Id='DownloadAndInvokeBootstrapper' Directory="INSTALLDIR" Execute="deferred" ExeCommand='powershell.exe -NoProfile -windowstyle hidden Invoke-WebRequest -Uri "https://go.microsoft.com/fwlink/p/?LinkId=2124703" -OutFile "$env:TEMP\MicrosoftEdgeWebview2Setup.exe" ; Start-Process -FilePath "$env:TEMP\MicrosoftEdgeWebview2Setup.exe" -ArgumentList (&apos;/install&apos;) -Wait' Return='check'/>

Or with silent installation (which i guess is preferred):

<CustomAction Id='DownloadAndInvokeBootstrapper' Directory="INSTALLDIR" Execute="deferred" ExeCommand='powershell.exe -NoProfile -windowstyle hidden Invoke-WebRequest -Uri "https://go.microsoft.com/fwlink/p/?LinkId=2124703" -OutFile "$env:TEMP\MicrosoftEdgeWebview2Setup.exe" ; Start-Process -FilePath "$env:TEMP\MicrosoftEdgeWebview2Setup.exe" -ArgumentList (&apos;/silent&apos;, &apos;/install&apos;) -Wait' Return='check'/>

Tested only on windows 10, but it should work on win7 too, not that it really matters when the download itself doesn't work 🤷

@lucasfernog lucasfernog force-pushed the feat/webview2-install-options branch from a3c4583 to ae2fa1f Compare June 26, 2022 13:33
@lucasfernog
Copy link
Member Author

@FabianLars I pushed the silent option and changed the default to be DownloadBootstrapper. But i tried the powershell change and I didn't see any benefits. In my Windows 11 sandbox it started showing the powershell window (it didn't before) and I couldn't see any difference with -Wait.

@FabianLars
Copy link
Member

Did you copy paste the line i posted as-is? Because for me (on win10) it doesn't show the powershell window and it did wait on the correct installer step.

I can't remember if i tested the version with the silent flag too, or only the passive variant, gonna retest.

Do you want to see a screen recording? :P

@lucasfernog
Copy link
Member Author

I tried it several times, only once it didn't show the powershell window. Weird.

@FabianLars
Copy link
Member

Or do you mean the really short >0.1second flash? Because that i can see, but it's the same for the old version.

@lucasfernog lucasfernog changed the title feat(bundler): extend webview2 installation options, closes #2882 feat(bundler): extend webview2 installation options, closes #2882 #2452 Jun 26, 2022
@lucasfernog
Copy link
Member Author

Yeah the flash is still there and I couldn't see the -Wait being effective - but that's probably just me being dumb.

@lucasfernog lucasfernog force-pushed the feat/webview2-install-options branch from da6af6d to 838cdeb Compare June 26, 2022 14:51
@lucasfernog
Copy link
Member Author

This PR is now ready. I wanted to prevent the powershell window flash but we can do that later if it's even possible.

@FabianLars
Copy link
Member

Just so we're on the same page, i recorded it without the silent option. The UAC prompt kinda messes up the recording, but it should be clear enough.

Old.Installer.mp4
New.Installer.-.Non-Silent.mp4

FabianLars
FabianLars previously approved these changes Jun 26, 2022
@lucasfernog
Copy link
Member Author

It's weird because on my sandbox the installer was also waiting for the webview installation to finish - but I'm glad it's consistent now.

@lucasfernog lucasfernog merged commit 2ca762d into dev Jun 26, 2022
@lucasfernog lucasfernog deleted the feat/webview2-install-options branch June 26, 2022 18:45
@dong0926
Copy link

I tested the installing of the new modes but i'm having issues running the apps in my VM :( probably too old The procedure entry point EventSetInformation could not be located in the dynamic link library advapi32.dll.

Has this problem been solved? I have the same problem

@dong0926
Copy link

@lucasfernog

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants