Summary:
Flipper used to always subscribe to the adb logs. This change makes the log subscription lazy, which means if there are no plugins listening to the logs, we don't subscribe to the adb logs at all. As you can see in the recording (prod build) this lowers CPU usage significantly: without logs plugin disabled it is now ~5-8 %, with the plugin enabled ~12-18%. Before this change we would never stop listening to the log output, even when the plugins were disabled, causing a constant background noise which people complain regularly about.
The pause / resume button in the new log plugin will now cleanup the connection as well, so that a simple 'pause' click will already boost performance, without needing to disable the logs plugin (if crash reporter is not enabled)
In this diff we also will clear the logs when reconnected. Previously we didn't reset the logs after connecting, we means Flipper would try to gets up with all past logs first, which could be a few 100 K entries.
Further future optimizations could involve using logcat smarter; by actually passing filters on app or loglevel to the command (not sure if that is actually faster, or just a convenience api that does the same filtering as we do)
Changelog: Flipper will now use less CPU if logs & crash reporter plugins are disabled by no longer tailing adb logcat.
Reviewed By: nikoant
Differential Revision: D27047041
fbshipit-source-id: 251a04dcc6f488f220cb56fe50a26788d795e38c
Summary:
This diff converts the CrashReporter plugin to Sandy. The main driver is that it allows us to fix the connection management of logs in a next diff.
There are few changes to highlight:
* A bunch of the old unit tests are removed, as they primarily verified that persistedState abstraction works, a concept that doesn't exist anymore with Sandy (as a result all the logic in error handling and crash reporter plugin has become a lot more trivial as well)
* Added additional unit tests to verify that the integration with notifications from Sandy, and the integration of crashes in combination with CrashReporter plugin works (this wasn't the case before)
* Plugin errors were always suppressed in production builds of Flipper. However, that makes error reporting pretty pointless in the first place, so enabled it by default, but made it a setting in case this results in too many errors suddenly.
* The integration with clicking OS crash notification -> bringing the user to a sensible place _doesn't_ work, but it didn't work before this diff either, so will address that later
* This doesn't upgrade the Crash reporter UI to sandy yet, will do that later in a separate diff
Changelog: Crash reporter will now report errors triggered from the device / client plugins by default. This can be disabled in settings.
Reviewed By: priteshrnandgaonkar
Differential Revision: D27044507
fbshipit-source-id: 8233798f5cce668d61460c948c24bdf92ed7c834
Summary:
Renamed actions "star" and "unstar" everywhere to "enable", "disable" and "switch". The logic behind original "star" action changed significantly, so this rename just makes everything much clearer.
Please note that as a part of rename persisted state fields "userStarredPlugins" and "userStarredDevicePlugins" were renamed. I've added a "redux-persist" migration for seamless transition.
Reviewed By: passy
Differential Revision: D26606459
fbshipit-source-id: 83ad475f9b0231194701c40a2cdbda36f02c3d10
Summary:
*Stack summary*: this stack adds ability to manage device plugins in the same way as client plugins: install, update, uninstall, enable (star) and disable (unstar) them.
*Diff summary*: implemented all plugin management actions for device plugins.
Changelog: it is now possible to enable/disable and install/uninstall device plugins
Reviewed By: mweststrate
Differential Revision: D26337377
fbshipit-source-id: 7d1ed61a8dc5f3339e5e548c613b67bca0c27f4f
Summary:
*Stack summary*: this stack refactors plugin management actions to perform them in a dispatcher rather than in the root reducer (store.tsx) as all of these actions has side effects. To do that, we store requested plugin management actions (install/update/uninstall, star/unstar) in a queue which is then handled by pluginManager dispatcher. This dispatcher then dispatches all required state updates.
*Diff summary*: refactored "star plugin" operation to perform it in pluginManager dispatcher.
Reviewed By: mweststrate
Differential Revision: D26305576
fbshipit-source-id: 90516af4e9ba8504720ddfa587f691f53e71b702
Summary:
*Stack summary*: this stack refactors plugin management actions to perform them in a dispatcher rather than in the root reducer (store.tsx) as all of these actions has side effects. To do that, we store requested plugin management actions (install/update/uninstall, star/unstar) in a queue which is then handled by pluginManager dispatcher. This dispatcher then dispatches all required state updates.
*Diff summary*: refactored Flipper mocking helpers to allow testing of plugin commands, and wrote some tests for pluginManager.
Reviewed By: mweststrate
Differential Revision: D26450344
fbshipit-source-id: 0e8414517cc1ad353781dffd7ffb4a5f9a815d38
Summary:
This diff introduces support for keeping clients around after they have disconnected. This is a pretty important debugging improvement, that will allow inspecting a device / app after it crashed for example.
With this diff, the current client is just kept around until it connects again, instead of throwing clients immediately away if they disconnect. After this change, ArchivedClients will only be created by imports / exports, and no longer by disconnects. Initially I played with improving the creation of archived devices, by migrating all plugin state over from the original client to the archive, but I discovered that is very prone, as it would be a lot of pointer redistribution (plugins would point to a different client / device etc). While in contrast, disconnected clients is already an existing concept in Flipper, so reusing that keeps all the changes relatively simple.
Note that we could potentially still reuse old clients around after reconnected, but it would become much harder to reason about how plugins would behave if they missed updates for a while, so throwing away the device / clients and starting with a fresh slate sounds safer. So I figured that chance to be too risky for now, but would probably be good follow up work.
Issues with import / export, UX, and making calls to to a disconnected client will be addressed in follow up diffs
Changelog: Clients will retain their state after being disconnected, until they reconnect again
Reviewed By: nikoant
Differential Revision: D26224677
fbshipit-source-id: feb9d241df2304341c2847fe7fd751ac54c045f6
Summary:
While trying to change something, discovered we have 3 different mechanisms in our code base to compute active plugins; the plugin list component, support form, and export flipper trace form had all their own, subtly different implementations of computing which plugins are available to the user.
Also removed some hardcoded exceptions for e.g. Logs plugin, which in the next diff and onward will be just a vanilla plugin without special casing
Unified that, which some how went a bit deeper than hoped, trough some hoops in in circular deps. Also unified to use the same testing utils, to avoid some gobbling objects manually together, with resulted in a bunch of unexpected NPEs. Found out that we actually still have unit tests using Flow in the process :-P. Converted one to TS.
Reviewed By: nikoant
Differential Revision: D26103172
fbshipit-source-id: 2fce2577d97d98543cb9312b3d013f24faee43aa
Summary:
Introduced API to replace the deprecated `selectPlugin` in Sandy.
The API can be used to navigate from `device plugin -> device plugin`, or` client plugin -> device / client plugin`
Introduced `isPluginAvailable` as well, so that the user interaction an be fine tuned in case the plugin is not disabled.
Reviewed By: jknoxville
Differential Revision: D25422370
fbshipit-source-id: c6c603f1c68e6291280b3d0883e474448754ded1
Summary: Tests to verify the more complex selection logic resulting from the Metro device exception. Also verifies the logic that computes the available plugins
Reviewed By: nikoant
Differential Revision: D24445555
fbshipit-source-id: 70110c4470e6aa1356e814aa40744b65c21cad89
Summary:
[interesting] since it shows how Flipper APIs are exposed through sandy. However, the next diff is a much simpler example of that
This diff adds support for adding menu entries for sandy plugin (renamed keyboard actions to menus, as it always creates a menu entry, but not necessarily a keyboard shortcut)
```
client.addMenuEntry(
// custom entry
{
label: 'Reset Selection',
topLevelMenu: 'Edit',
handler: () => {
selectedID.set(null);
},
},
// based on built-in action (sets standard label, shortcut)
{
action: 'createPaste',
handler: () => {
console.log('creating paste');
},
},
);
```
Most of this diff is introducing the concept of FlipperUtils, a set of static Flipper methods (not related to a device or client) that can be used from Sandy. This will for example be used to implement things as `createPaste` as well
Reviewed By: nikoant
Differential Revision: D22766990
fbshipit-source-id: ce90af3b700e6c3d9a779a3bab4673ba356f3933
Summary:
Device plugins have an activate and deactivate hook, that reflects the plugin being selected in the UI. Added these same hooks to client plugins as well. In practice they are called at the same times as `onConnect` and `onDisconnect`, except for background plugins, which connect only once, so it is pretty useful to be still able to make the distinction.
Since there is now quite some common functionality between plugins and device plugins, will clean things a bit up in a next diff
[Interesting] as it explains the difference between the different lifecycle methods of plugins, and the impact of being a background plugin
LIfecycle summary:
1. app connects
2. for background plugins: connect them (`onConnect`)
3. user selects a plugin, triggers `onActivate`. will also trigger `onConnect` the plugin if it _isnt_ a bg plugin
4. user selects a different plugin, triggers `onDeactivate`. will also trigger `onDisconnect` if it isn't a bg plugin.
5. app is unloaded. Triggers `onDisconnect` for bg plugins. Triggers `onDestroy` for all plugins,
Reviewed By: jknoxville
Differential Revision: D22724791
fbshipit-source-id: 9fe2e666eb37fa2e0bd00fa61d78d2d4b1080137
Summary: Now that we can load device plugins, let's make sure they are rendered as well.
Reviewed By: passy, nikoant
Differential Revision: D22693927
fbshipit-source-id: 22574ec6e629e6dd66e42193b406ceb7dfcf1836
Summary:
This diff makes sure that devices will actually instantiate applicable sandy device plugins. Similar to how client plugins are owned by Client, device plugins are directly owned by BaseDevice, which significantly simplifies life cycle management and doesn't dispatch updates to all Redux connect components whenever something irrelevant changes.
Also made sure `device.teardown()` is called. That API already existed, but wasn't used or implemented before.
Updated Flipper test utils to support testing device plugins as well (both Sandy and classic ones)
Reviewed By: passy, nikoant
Differential Revision: D22693929
fbshipit-source-id: 73b2b8666ef7a0e748ea89360db84734d37eb5be
Summary:
Created a few breakages. Would appreciate some closer eyes on this.
Pull Request resolved: https://github.com/facebook/flipper/pull/1366
Reviewed By: mweststrate
Differential Revision: D22501454
fbshipit-source-id: 9b882a12aecc65da85f29101bf87bf27519a7d2a
Summary: While testing manually discovered the sandy plugin infra din't cover the case that a plugin can be selected but not enabled at the same time. Added test and fixed that.
Reviewed By: nikoant
Differential Revision: D22308597
fbshipit-source-id: 6cef2b543013ee81cee449396d523dd9a657ad1c
Summary:
While writing unit tests discovered a bug that disabling a plugin doesn't guarantee cleaning the messagequeues (both the buffer in client and the messagequeue reducer).
Fixed that. That was thanks to @#%@#$@#%@ Redux a lot harder than it should be; as 'STAR_PLUGIN' reasons about a plugin + app name, while the message queue reducer would need to deduct the plugin keys from that, but it can't because that mapping is stored in the connections reducers. So I moved the `STAR_PLUGIN` action handling to the root reducer, sot that it can reason about the state of multiple reducers, which looked like the least of all evils. For more ranting about that and possible alternative solutions: https://twitter.com/mweststrate/status/1277556309706117122
Reviewed By: nikoant
Differential Revision: D22284043
fbshipit-source-id: 35d0a8ba3a21a5959d2bb6ef17da3ff5077f48fd
Summary: Sandy plugins can now send messages to plugins. The methods and params are strongly typed in implementation and unit tests, based on the <Methods> generic of FlipperClient.
Reviewed By: nikoant
Differential Revision: D22256972
fbshipit-source-id: 549523a402949b3eb6bb4b4ca160dedb5c5e722d
Summary:
This sets up the initial infra that is to be used by plugin devs to test plugins.
There is not much yet to see, as there is no state or message sending yet. But at least the life cycle of plugins can be test, things are strongly typed and everything is in the place where it should be :)
N.b. the import difference with these utils and the createFlipperMock utilities in Flipper are
1. this testing infra is entirely inside flipper-plugin package, so that plugin devs don't need flipper as a dependency
2. this testing infra doesn't provide abstractions for plugin / device / client switching; it tests plugins purely in isolation of the rest of the world (except for firing `onConnect` / `onDisconnect` which is normally the effect of switching plugins)
Reviewed By: nikoant
Differential Revision: D22255262
fbshipit-source-id: b94ccbab720d2b49428a646aed3c55af71a5bc80
Summary:
Introduced hooks that are called whenever the plugin is connected / disconnected to it's counter part on the device.
There is some logic duplication between `PluginContainer` for old plugins, and `PluginRenderer` for new plugins, mostly caused by the fact that those lifecycles are triggered from the UI rather than from the reducers, but I figured refactoring that to be too risky.
Reviewed By: jknoxville
Differential Revision: D22232337
fbshipit-source-id: a384c45731a4c8d9b8b532a83e2becf49ce807c2
Summary: PluginContainer will now wrap Sandy plugins in PluginRenderer. PluginRenderer will also be used by plugin unit tests in the future
Reviewed By: jknoxville
Differential Revision: D22159359
fbshipit-source-id: 69f9c8f4bec9392022c1d7a14957f5aca0339d97
Summary:
This diff makes sure sandy plugins are initialized.
Sandy plugins are stored directly in the client for two reasons
1. we want to decouple any plugin state updates from our central redux store, as redux is particularly bad in handling high frequency updates.
2. The lifecycle of a plugin is now bound to the lifecycle of a client. This means that we don't need 'persistedStore' to make sure state is preserved; that is now the default. Switching plugins will no longer reinitialize them (but only run specific hooks, see later diffs).
3. PersistedState will be introduced for sandy plugins as well later, but primarily for import / export / debug reasons.
A significant difference with the current persistent state, is that if a client crashes and reconnects, plugins will loose their state. We can prevent this (again, since state persisting will be reintroduced), but I'm not sure we need that for the specific reconnect scenario. Because
1. we should fix reconnecting clients anyway, and from stats it looks to happen less already
2. reconnects are usually caused by plugins that aggregate a lot of data and get slower over time. Restoring old state also restores those unstabilites.
For the overview bringing back the archi picture of earlier diff:
{F241508042}
Also fixed a bug where enabling background plugins didn't enable them on all devices with that app.
Reviewed By: jknoxville
Differential Revision: D22186276
fbshipit-source-id: 3fd42b577f86920e5280aa8cce1a0bc4d5564ed9
Summary: The test mock utilities now return a promise, rather than taking a callback, which makes tests slightly nicer to read (similar to react-testing-library). No semantic changes.
Reviewed By: jknoxville
Differential Revision: D22186278
fbshipit-source-id: ec5b9f4e6bfeee9160e331f8c20a1d4fdcbfeede
Summary:
There were several reports that it is hard to test an entire plugin, and send updates.
This should address that and help with avoiding plugin regressions in the future.
Added a `renderMockFlipperWithPlugin` utility and demo test.
Reviewed By: passy
Differential Revision: D22114217
fbshipit-source-id: ceefd954abc4ea47c336eab495fb50f161993df9
Summary:
Before we dropped all messages if the client connected before the device has been registered. Which happens typically on iOS (it can actually take very long, will investigate that as well, but not in this diff).
However, we don't need to wait for the device to process messages, we just need its serial, which we already know (given that we use that query id to find the matching device in the first place).
I think this will greatly improved perceived stability for iOS
Reviewed By: jknoxville
Differential Revision: D20734892
fbshipit-source-id: f98e8d31558ef606b9a8287e03fc41ab6c3a087d
Summary:
1) moved "sonar/desktop/src" to "sonar/desktop/app/src", so "app" is now a separate package containing the core Flipper app code
2) Configured yarn workspaces with the root in "sonar/desktop": app, static, pkg, doctor, headless-tests. Plugins are not included for now, I plan to do this later.
Reviewed By: jknoxville
Differential Revision: D20535782
fbshipit-source-id: 600b2301960f37c7d72166e0d04eba462bec9fc1
