Skip to content
This repository was archived by the owner on Oct 22, 2024. It is now read-only.

desktop popup poc#532

Merged
cryptotavares merged 31 commits intomainfrom
poc/desktop-popup
Mar 8, 2023
Merged

desktop popup poc#532
cryptotavares merged 31 commits intomainfrom
poc/desktop-popup

Conversation

@cryptotavares
Copy link
Copy Markdown
Contributor

@cryptotavares cryptotavares commented Feb 15, 2023

Overview

Add a desktop popup window. The main motivation for that is performance and an improved desktop experience. Why is it performant? Because on the desktop we can have the popup (approval window) loaded and ready since the moment that the app is initialised. We just hide or display and route accordingly whenever the popup needs to be displayed to the user.

Changes

Root

Two new environment variables:

  • DESKTOP_POPUP -> this enables the desktop popup.
  • DISABLE_EXTENSION_POPUP -> this one disables the extension popup (but only if the desktop popup is enabled.. there is no path where no popup will be shown)

Patch:

  • webextension-polyfill -> this package is only meant to be loaded within an extension environment. Although electron allows loading an extension, we do not want to load the entire MetaMask Extension within electron. We just want to load part of the MetaMask Extension popup. And to do that we are actually creating a different web app within an electron BrowserWindow. As BrowserWindow does not have chrome.runtime global, we had to patch webextension-polyfill to create the required polyfills even if we are not in an extension environment.

Init Extension submodule:

  • React - remove react deps from the extension submodule node_modules and link them with the react deps on the app node_modules. This is to prevent the having more than one copy of react in the same app issue. (Also bear in mind that this is just required for the Desktop App.
  • webextension-polyfill - replaced the dep from the extension submodule node_modules, using the same strategy used for react. The reason for this one, is that we need to run the patched version when on the desktop popup, but on the extension want to run the version without the patch.

Pipeline

Updated the Circle pipeline, so that the Extension built for the e2e tests does not take into account the linked deps from the App. This allows the Extension to be built with lavamoat enabled and use the policies from the submodule.

App

Desktop popup

This is the actual desktop popup ui react app. It is loaded within an electron browserWindow. The way that it works is super easy.

UI

The ui is quite standard and mimics the structure of the desktop ui. The main differences are:

  • the redux store is exactly the same as the MetaMask Extension redux store
  • to send the requests to the background script (background is running on desktop main process), we have actually wrapped the electronBridge in a stream and pass that into the MetaRPC client.
  • we added a loading page that is the default one and implemented the same logic that the MetaMask Extension default route has in order to decide if the popup should be closed or not (as the popup routes will redirect to the default one up on approval or rejection).
  • it is also worth mentioning a browser-shim that was added mainly because the popup-ui is loaded within a browserWindow, and not within a browser extension environment.
    Build process:
    To generate the bundle we have reused the same scripts that we use for desktop-ui. In fact we treated popup-ui as another entry point, but the commons are exactly the same. This means that most of the bundle is reused by both apps. Finally we have also split the css bundling. The dist will now include a popup-index.css file that will be loaded by the popup-ui.html file

Connection with Desktop App

It establishes a connection using the connectRemote but by passing our own duplex stream (called approvalStream). That approval stream is actually just a stream wrapping the popup-ui browser window webContents.send method. So to sum up we are:

  • wrapping the ipcRender msg channel into a stream on the popup-ui and also on the main process.
  • on the ui we create the background stream by
    • wrapping the electron bridge with a stream
    • passing that stream into the metaRPC client factory
  • on the main process we create the communication stream by
    • wrapping the ipcMain and the popup browser window with a stream
    • passing that stream into the MetaMask Extension ConnectRemote

Desktop App

Besides establishing the connection, the main process is also responsible for:

  • creating the popup window
    • the window is created whenever the DESKTOP_POPUP env var is set to true. It will load the popup-ui.html (or the dark version, depending on the current theme). And it will have its own context bridge (loaded using the popup-preload.js)
  • managing when that window should be visible or hidden
    • the Extension background script is responsible to launch the popup. It is also responsible (in some cases) to close the popup. In order to do the same for the desktop popup, we are adding window handlers that update the desktop popup browser window visibility instead of proxying those requests to the actual paired extension.

@cryptotavares cryptotavares force-pushed the poc/desktop-popup branch 3 times, most recently from 1aff5c1 to 208c8df Compare February 22, 2023 18:29
@cryptotavares cryptotavares force-pushed the poc/desktop-popup branch 5 times, most recently from 34d2cdc to cc08a0c Compare February 24, 2023 17:53
@cryptotavares cryptotavares marked this pull request as ready for review February 24, 2023 17:54
@cryptotavares cryptotavares requested a review from a team February 24, 2023 17:54
@cryptotavares cryptotavares changed the title DRAFT Poc/desktop popup desktop popup poc Feb 24, 2023
"vinyl-sourcemaps-apply": "^0.2.1",
"wait-on": "^7.0.1",
"watchify": "^4.0.0",
"webextension-polyfill": "^0.8.0",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it was duplicated as a dev and prod dep... removed the dev dep

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 7, 2023

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@cryptotavares cryptotavares force-pushed the poc/desktop-popup branch 2 times, most recently from 3808a1a to 1f855e9 Compare March 7, 2023 19:03
matthewwalsh0 and others added 12 commits March 7, 2023 19:10
Show and hide window using background controller listeners.
electron BrowserWindows do not
have the property chrome.runtime.
In fact, unless we are loading an
extension, it will not have any of
the browser extension API properties.
In order to polyfill those we are
relying on the webextension-polyfill
package. But unlike the background
process, we do not have chrome.runtime
within electron Browser window (as there
is no extension id, because we are not
loading an extension). We are patching
webextension-polyfill to bypass checking
for chrome.runtime and chrome.runtime.id
Notification manager expects
a window object with left prop
to be returned when creating
a popup window.
Replicate the behaviour from
home component on the extension.
Confirmation and permission pages
route to the '/' (home) page. And
then the home page decides if the
notification popup window should be
closed or not.
There are a couple of actions
that call methods from global.platform.
Those are not available in the electron
BrowserWindow and we do not need them.
To prevent unhandled promise rejections
we are overriding those to noop
We need lavamoat policies when
the extension react deps are symlink
to the app deps
Conscious ignoring popup-ui
as it was causing issues due to
imports from the extension submodule.
And we are not using TS on the popup-ui
yet.
done

rm -rf $EXTENSION_DIR/node_modules/@lavamoat/aa
ln -s ../../../../node_modules/@lavamoat/aa/ $EXTENSION_DIR/node_modules/@lavamoat/aa
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not essential but we could combine all these into a single loop and remove the duplication if we create a list first with all the deps (react, webextension, and @lavamoat/aa).

@cryptotavares cryptotavares merged commit c2c71d1 into main Mar 8, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 8, 2023
@cryptotavares cryptotavares deleted the poc/desktop-popup branch May 15, 2023 16:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants