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

In-process components on ARM64 #9216

Merged
merged 6 commits into from
Jan 31, 2019
Merged

In-process components on ARM64 #9216

merged 6 commits into from
Jan 31, 2019

Conversation

jcsteh
Copy link
Contributor

@jcsteh jcsteh commented Jan 31, 2019

Link to issue number:

None.

Summary of the issue:

Firefox is coming to Windows 10 on ARM64! In order for NVDA to access this (as well as other browsers and apps that need in-process helpers), it needs to include in-process components built for ARM64.

Description of how this pull request fixes the issue:

  1. Upgrade SCons to version 3.0.4, which includes necessary changes to support building for ARM64.
  2. Build, install and use in-process components for ARM64.
  3. Fix AppModule.is64BitProcess for ARM64. Previously, it always returned True, even for 32 bit processes. This was also breaking support for 32 bit SysListView32 controls (including those in NVDA's own GUI) on ARM64 systems.

Testing performed:

  1. With a local build of Firefox with accessibility enabled, tested that about:support works in browse mode. (Nightly builds currently have accessibility disabled, hence the local build. We can't test real web documents yet because accessibility currently causes immediate content process crashes on ARM64. However, from NVDA's perspective, there shouldn't be any more to do once that bug is fixed.)
  2. Opened notepad. With the NVDA Python console, tested that nav.appModule.is64BitProcess returns True.
  3. Opened the NVDA Python console with NVDA+control+z, then pressed NVDA+control+z again so that nav is within NVDA itself. Tested that nav.appModule.is64BitProcess returns False.
  4. Opened the NVDA Preferences dialog. Tested that the names of the categories are read when cursoring up and down the list. (Previously, they weren't.)
  5. Opened Notepad. Verified that screen review only shows blank (expected because display model is disabled; see below), but that there were no exceptions.

Known issues with pull request:

  1. Display model is disabled because MinHook does not support ARM64. I don't anticipate this being an issue because I would think an app being built for ARM64 would be modern enough that it wouldn't rely on GDI. Regardless, it's outside the scope of this PR.
  2. I haven't updated the Mozilla symbols upload script. Firefox crash reporting isn't working on ARM64 yet anyway. Once it is, I might look into this, but that can be done in a separate PR.
  3. This does not build in-process components for ARM32. I'm not sure anyone will bother shipping ARM32 desktop apps (if this is even supported). Again, this is out of scope here.

Change log entry:

Section: New features, Changes, Bug fixes

- Early support for apps such as Mozilla Firefox on computers with ARM64 (e.g. Qualcom Snapdragon) processors.

Note: You may want to mention other apps here, but I don't know what plans are for other apps like Chrome and I haven't tested these.

@jcsteh
Upgrade SCons to version 3.0.4.
8652d50 
This required some tweaks to the sconscripts for ISimpleDOM and MathPlayer.
We copy the idl file into the build directory ourselves, but it depends on other files.
It seems SCons no longer scans for dependencies in files we copy ourselves.
Thus, we must explicitly declare those dependencies.
@jcsteh
Build and install in-process components for ARM64.
a417b98 
These are placed in a new libArm64 directory (alongside lib and lib64).
MinHook doesn't support ARM64, so disable MinHook and everything that depends on it (notably displayModel).
@jcsteh
Start nvdaHelperRemoteLoader appropriately on ARM64.
8d1b1f9
@jcsteh
Fix AppModule.is64BitProcess for ARM64.
e730ce3 
Previously, it always returned True, even for 32 bit processes.
On ARM64, isWow64Process always returns False.
We must instead use IsWow64Process2 where supported.
@jcsteh
Update readme to list the additional Visual Studio components we now …
4f5e8b3 
…require.
@josephsl
Copy link
Collaborator

josephsl commented Jan 31, 2019 via email

@michaelDCurran michaelDCurran merged commit ec49fa7 into nvaccess:master Jan 31, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.1 milestone Jan 31, 2019
@jcsteh jcsteh deleted the arm64Inproc branch January 31, 2019 05:47
@LeonarddeR
Copy link
Collaborator

I wonder whether some vbuf backends could be also left out, since building them doesn't make much sense for ARM64. This includes:

  • adobeFlash
  • lotusNotesRichText

Unless we'll have native ARM64 editions of itunes and Adobe Reader in the future, these backends could be left out as well. I guess it doesn't have much impact though, the most noticeable would be a slightly improved building time.

@LeonarddeR
Copy link
Collaborator 

1. Display model is disabled because MinHook does not support ARM64. I don't anticipate this being an issue because I would think an app being built for ARM64 would be modern enough that it wouldn't rely on GDI. Regardless, it's outside the scope of this PR.

For reference, Microsoft Detours supports ARM64 and it is now open source, so if we ever need hooking there, we probably have to switch.

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

@michaelDCurran michaelDCurran michaelDCurran approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2019.1
Development

Successfully merging this pull request may close these issues.
5 participants
@jcsteh @josephsl @LeonarddeR @michaelDCurran @nvaccessAuto