Summary:
Background for this diff: https://fb.quip.com/KqEfAlKYlgme
Some plugins don't respect that stuff (livefeed and graphql), but for others it seems to work fine.
This is just a PoC, there are some present bugs concerning the combination of selecting and bg plugins
Questions to investigate:
- [x] make sure that LiveFeed and GraphQL disconnect properly. There might be more plugins that need that
- [x] verifiy that we don't loose one of the original goals of background plugins, e.g. QPL collecting and sending data from device start. Does this still work as intended after this change?
- [x] how can we observe / measure improvements? Are dev builds more responsive after this? Is the layout inspector smoother for example because no QPL plugins are interweaved?
- [x] how is forward and backward compatibility?
- If Flipper is updated, but device not: No change I think, as getBackgroundPlugins() will return an empty set, and background plugins are initiated as usual, so old behavior
- If device is updated, but Flipper not, background plugins won't be started until they are selected. This is a degradation, but hopefully explainable.
- [x] Verify QPL buffer is not unbounded
- [x] Share architecutre changes with team
For Graphql updates: D20943455
Added runtime stats to monitor network traffic (sadly had to redo that since scuba couldn't handle the data format used at first, so probably will hold of landing this diff a week to make sure we can see some effects)
Follow up work:
[x] wait until we released the stat tracking before we release this, to be able to measure the effect?
[x] make sure graphql fix lands
[ ] use side effects abstraction
[ ] fix other background plugins (android only) or fix it in a generic way:
{F234394286}
Changelog: Background plugins will no longer receive a Flipper connection if they are disabled. This should significantly reduce the overall load of Flipper both on the device and desktop when unused plugins are disabled used, which could otherwise generate 10MB/s of network traffic certain scenarios. All plugins *should* be able to handle to this gracefully, but since this is quite a fundamental change, reach out to the Flipper team when in doubt!
Reviewed By: jknoxville
Differential Revision: D20942453
fbshipit-source-id: b699199cb95c1b3e4c36e026b6dfaee7d1652e1f
Summary:
Context: https://fb.workplace.com/groups/flippersupport/permalink/803808233433170/
A similar change was done earlier with the secure connection route.
This does the same for the insecure route.
Reviewed By: mweststrate
Differential Revision: D21227196
fbshipit-source-id: 844cb674b5b16033977f451bbc3d8bbc69732273
Summary:
Originally, Flipper would use exceptions for the control flow of determining if there's no desktop client available. It would catch an exception and then kick off another attempt. This is cool, but if you are debugging an application where Flipper is enabled, and you set a breakpoint to detect all thrown exceptions, then you'll see an exception thrown inside Flipper logic every couple of seconds. That makes it hard to debug what you're trying to do.
There's a workaround where you can simply open Flipper, but that's a bit annoying.
This diff changes the logic in Flipper to not use exceptions for the part of connection where it's doing the initial connect. We could apply this to `doCertificateExchange` as well, but that's actually an error there so I don't see the need to do it there as well.
Reviewed By: jknoxville
Differential Revision: D21016737
fbshipit-source-id: cbdfe2794b4d38a9e3b8304ebee845655bb26ae5
Summary:
This is in line with the most recent stable Android Studio Release.
Pull Request resolved: https://github.com/facebook/flipper/pull/958
Test Plan:
Used it myself.
Open Source CI required a higher NDK, so let's first check what CI says to that internally now.
Reviewed By: jknoxville
Differential Revision: D20794634
Pulled By: passy
fbshipit-source-id: c32f934634b036ad3c1cad9fc49541e585d64329
Summary:
The problem is that whenever an app is shutdown, and then reopened, the flipper dir gets reset when getting the CSR for connecting to flipper.
This causes the first connection attempt to fail always, and it goes through the whole cert exchange, taking longer than necessary.
Fixes it by loading the csr from disk if it's not loaded yet, without blowing away the whole certs state.
A side effect of this would be that as long as some file exists where the csr lives, flipper state would never get reset, so it wouldn't be able to fix itself automatically anymore. To keep that working, I've made `resetFlipperDir()` public and am calling it explicitly when starting certificate exchange. This should ensure that we still reset when we need to, but not unnecessarily.
The reason it went wrong is that getCSR used to be called only at cert exchange, when resetting and generating a new one was always desirable. However, when we shipped the fix for changeable android serials, it started to be used as a normal getter.
Reviewed By: timur-valiev
Differential Revision: D18834806
fbshipit-source-id: 56ca7e03e1aa9011f836bc9c021cf3048f7dc1e4
Summary:
There was an interesting problem in Flipper which I encountered. On closing our app we would deadlock. This was happening when joining the Flipper thread.
Upon investigation I found that it happened only when Flipper was not connected to the app. And digging in I found that it's because we are continually trying to connect the `FlipperClient` to the Flipper app when there is no connection. This works great, but it continues even if you have asked Flipper to terminate. So this meant that the `EventBase` never gets chance to shutdown, as it keeps getting new work to do.
The fix is to add a flag to `FlipperConnectionManagerImpl` to say if the conection is started or not. Then, if it's not started and we get a request to retry connection, then we will fail it.
Reviewed By: jknoxville
Differential Revision: D18780622
fbshipit-source-id: cce82cbb5c54e6d92ea16644c8a247bd2578ae26
Summary: in FlipperRSocketResponder::handleFireAndForget, a responder object was being conditionally (for requests with an "id" field) created, resulting in a null pointer passed into FlipperClient::onMessageReceived when no id was present. FlipperClient::onMessage received, in the meantime, unconditionally dereferences that pointer, resulting in a segmentation fault. This change creates the responder object regardless of whether or not the "id" key is present in the request object, thus avoiding passing a null pointer into FlipperClient::onMessageReceived.
Reviewed By: jknoxville
Differential Revision: D18583898
fbshipit-source-id: 2112c45bc0cd639cec908d0039d6bdaed2f61491
Summary: FlipperState may be updated from multiple threads, so it needs synchronization. Note the comments in the diff about why we drop the lock before calling out to the update listener -- this can be changed if necessary in the future.
Reviewed By: jknoxville
Differential Revision: D18095471
fbshipit-source-id: 95d558394ae1a9b7583e5a61969e1eeda6448cff
Summary:
This avoids having to perform gymnastics like in
https://github.com/facebook/react-native/pull/26697
Reviewed By: jknoxville
Differential Revision: D17735954
fbshipit-source-id: 507548aab89309beeb228b104a24af8acd10ce9a
Summary:
D17629896 was intended to fix this in ovrsource, but it turns out these changes should be made on fbsource first then get synced.
The MSVC build of OculusPCSDK has numerous warnings, including these low-hanging fruit:
```
c:\open\ovrsource\xplat\omnistore\client\common\reportsubscriptionstatetiming.cpp(28): warning C4101: 'e': unreferenced local variable
c:\open\ovrsource\xplat\omnistore\client\common\databaseanalyticsmetadatatiming.cpp(23): warning C4101: 'e': unreferenced local variable
c:\open\ovrsource\xplat\omnistore\client\common\sendqueuereportbacklogtiming.cpp(32): warning C4101: 'e': unreferenced local variable
c:\open\ovrsource\xplat\omnistore\client\common\omnistore.cpp(192): warning C4101: 'e': unreferenced local variable
c:\open\ovrsource\xplat\omnistore\client\common\omnistore.cpp(907): warning C4101: 'e': unreferenced local variable
c:\open\ovrsource\xplat\omnistore\client\common\omnistore.cpp(934): warning C4101: 'e': unreferenced local variable
c:\open\ovrsource\xplat\omnistore\client\common\omnistore.cpp(946): warning C4101: 'e': unreferenced local variable
```
Clang doesn't complain, but the code is just as clear without the 'e' so best to remove.
Reviewed By: vener91
Differential Revision: D17631747
fbshipit-source-id: 0190a48e640311b40c9d1b988b0c07cfbdcfd7e5
Summary:
Background plugins are initialised when flipper connects.
But there's no isolation, so if any fail to initialize, then we don't carry on initializing the rest.
This fixes that by skipping failed ones only.
Reviewed By: passy
Differential Revision: D17600854
fbshipit-source-id: 2ef04cbcecda78ca448c2f4d56639ba63792e2f7
Summary:
- Add CSR and CSR destination to request body
- (unrelated) Rearrange the list of include files
Reviewed By: jknoxville
Differential Revision: D17346429
fbshipit-source-id: ce775998884b73e82a931f4dd8986c659a892b51
Summary:
When an operation fails, we have an error message. This was being displayed in the diagnostic screen output, but not included in the debug log.
This makes them both have same bahaviour, so they both include the error messages.
Reviewed By: passy
Differential Revision: D17181847
fbshipit-source-id: 823f73d641d2315da86a0019479716852950f9a5
Summary:
Pull Request resolved: https://github.com/facebook/flipper/pull/538
Upgrades libevent to the August release 2.1.11.
Some notable things:
- The CMake file they ship with is actually broken. It contains a reference to an "uninstall.cmake" script that they don't include in the archive.
- There is some git magic in the CMake script which throws very annoying warnings if you're not in a git repository and if you are, it's still super useless because it assumes it's the libevent repo.
- I've removed the uninstall reference and replaced version detection magic with hardcoded values. The result is the "CMakeLists.txt" in the overrides folder. For future upgraders: You'll have to either remove the override or apply similar steps to the next release.
- The `event-config.h` is simply one of the outputs I found under `android/build/libevent/x86/include/event2/event-config.h`. They seem to be stable across architectures so it's easier to just copy it over than to fix the CMake logic that should actually pick it up from the Android build path.
- The cmake target names have changed and have an underscore in them, now.
- Verified with an SDK 28 x86_64 that this does fix https://github.com/facebook/flipper/issues/482.
Fixes GH482.
Reviewed By: jknoxville
Differential Revision: D17164731
fbshipit-source-id: 642744118065bea2674dbb0e1af91a11598066cc
Summary:
Don't fatal when trying to send a message to flipper that is too large.
Instead just continue to throw an exception, so the plugin will still be aware of the failure.
Reviewed By: cpojer, danielbuechele
Differential Revision: D17180334
fbshipit-source-id: 49c341a7cee579158be3878639083c6d19b3eaa3
Summary:
In tests, the instance is never initialized and it's hard to guard against null when the
instance isn't initialized to nullptr (instead being random data).
Reviewed By: ximyu
Differential Revision: D16984717
fbshipit-source-id: 414539e1117abaad16df9f9b7d3b8dbb328e38d0
Summary:
Pull Request resolved: https://github.com/facebook/flipper/pull/463
Was made aware [here](https://github.com/facebook/flipper/issues/461) that
we currently don't build with NDK 20 because of Folly. The above issue exists only for x86 platform and not for other platform. I have updated the folly and RSocket. Although this diff doesn't solve the issue mentioned above for x86, but it updates the dependencies to the latest version. To mitigate temporarily, I have updated the gradle to not build for x86.
Reviewed By: passy
Differential Revision: D15713205
fbshipit-source-id: 79dd5825b0b876c87a1868fde8f3353a76012267
Summary:
`/*` is the standard throughout open source code. For example, Firefox uses single /*: https://hg.mozilla.org/mozilla-central/file/21d22b2f541258d3d1cf96c7ba5ad73e96e616b5/gfx/ipc/CompositorWidgetVsyncObserver.cpp#l3
In addition, Rust considers `/**` to be a doc comment (similar to Javadoc) and having such a comment at the beginning of the file causes `rustc` to barf.
Note that some JavaScript tooling requires `/**`. This is OK since JavaScript files were not covered by the linter in the first place, but it would be good to have that tooling fixed too.
Reviewed By: zertosh
Differential Revision: D15640366
fbshipit-source-id: b4ed4599071516364d6109720750d6a43304c089
Summary: I hope that this takes care of all the android violations.
Reviewed By: danielbuechele
Differential Revision: D15199150
fbshipit-source-id: 0f5b4edc1bc9d38de0885f88bb2c3e43acfa09cd
Summary:
Removed the symlink and moved `Flipper.podspec` and `FlipperKit.podspec` to the root directory. Also published `Flipper.podspec` and `FlipperKit.podspec`on cocoapods
http://cocoapods.org/pods/Flipperhttp://cocoapods.org/pods/FlipperKit
Also removed the stub podspec files.
Released Flipper and FlipperKit on Cocoapods
Pull Request resolved: https://github.com/facebook/flipper/pull/423
Reviewed By: passy
Differential Revision: D15146621
Pulled By: priteshrnandgaonkar
fbshipit-source-id: 86ee16773a81b0896548169cba49810250122ec1
Summary:
== Highlights
* Improved UI when exporting Flipper data. (45999a5292)
== Fixes
* The Shared Preference Plugin on Android will search for all shared pref files instead of only the package name by default. (825ecb8e23)
* When restarting Flipper and the previously selected plugin no longer exists, the UI would crash. (c4a8e73543)
Commit log: https://github.com/facebook/flipper/compare/v0.19.0...v0.20.0
Reviewed By: danielbuechele
Differential Revision: D14951753
fbshipit-source-id: dbafc2d41cd25b2ede218b3fe3e99745226939a9
Summary: Its helpful to store all history of sent messages for testing purposes.
Reviewed By: jknoxville
Differential Revision: D14797191
fbshipit-source-id: 0e58166cb4ab79852165f2107b2f6bcd259f1096
Summary:
Fixes the iOS OSS job which was failing. The root cause of the problem is as follows:
- RSocket was referring the master branch of rsocket/rsocket-cpp, fixed it by forking it and making its own release, unfortunately they don't make a release.
- Folly referred to the January's release. The current master branch of RSocket doesn't build with the latest release of the Folly, [this commit](b51dc569eb) is responsible for the breakage. Thus I made a Rsocket release, right before this [commit](b51dc569eb), so that it builds.Although, the current master branch of Folly builds with the current master branch of rsocket, but we will have to wait on Folly team to make a release. Once they do that I will update bit RSocket and Folly releases.
Reviewed By: jknoxville
Differential Revision: D14668686
fbshipit-source-id: 6b0362632c26355a69a3f33a031b71c88e7dd45b
Summary:
== Highlights
- **Layout 2.0**: We have a new layout inspector that was rewritten from the ground to address all sorts of small bugs that like nodes not expanding that should all be gone now.
- **New Crash Reporter**: Flipper will now detect crashes on your device and give you easy access to them through a device plugin that you can find next to logs. Learn more at https://fbflipper.com/docs/crash-reporter-plugin.html.
- **Import/Export**: You can now import and export Flipper sessions as data to share them with others. Learn more at https://fbflipper.com/docs/share-flipper-data.html.
== Fixes
- Improved error reporting for what was previously summarised under "RecurringError". (bf9be74ad2)
- Fixed an issue where the color picker could get cut off. (f9ec0cd326) (https://github.com/facebook/flipper/issues/396)
Full list: https://github.com/facebook/flipper/compare/v0.18.0...v0.19.0
Reviewed By: priteshrnandgaonkar
Differential Revision: D14665281
fbshipit-source-id: 78ed042d38642356180f25455d0d17a03ed59c6b
Summary:
= Highlights
This is a bit of a boring release, to be frank. Most of the stuff happened behind the scenes, but a lot of preparation for interesting stuff happened behind the scenes, so stay tuned!
= Fixes
- Upgraded to Electron 4.0.8 (e0127b8438)
- Fixes to node expansion in the layout inspector.
- Connectivity improvements.
See https://github.com/facebook/flipper/compare/v0.17.1...v0.18.0 for reference.
Reviewed By: jknoxville
Differential Revision: D14477605
fbshipit-source-id: 0d927ba08fa5fab9d95c76ec763f5b9dd5bc104d
Summary:
== Bugfix Release ==
Fixes#394 in which using the network inspector can cause the plugin view to crash. Also fixes an issue where native background plugins can fail to connect (d720b20870).
Reviewed By: danielbuechele
Differential Revision: D14384823
fbshipit-source-id: 7cee49d22ec2ed8f076cf3aeb980d4ba3a273a3f
Summary:
The graphql plugin gets injected after the client is connected. In this case, earlier we didn't use to call `didconnect` assuming that it will be called when the plugin gets clicked by the user in the UI. But that is not true anymore as GraphQL plugin is a background plugin, And we don't call `didConnect` for the background plugins on the user click in the UI, as we assume that background plugins should have already received the call before.
This diff fixes the issue by calling didConnect on the background plugin which gets added after the `FlipperClient` is initialised.
{F152780726}
Look at the bug:
Reviewed By: danielbuechele
Differential Revision: D14369587
fbshipit-source-id: f75bf4e4463a31fa4735e90245c46632858a32b7
Summary:
- The Android library now uses AndroidX instead of the support-library.
- Folly and RSocket dependencies uploaded to cocoapods (f8bd473828).
- Improved Unicode support in the Network Inspector plugin (f4b2ce9a2b).
- Fixed MacOS release creation.
- Several UI fixes regarding in-app search and the toolbar.
Reviewed By: passy
Differential Revision: D14269567
fbshipit-source-id: 2d2f965316bd1ddfb375a2db48fb715095693c43
Summary: Since plugins aren't under our control, they might throw exception pointers. We can still extract the useful info in this case.
Reviewed By: passy
Differential Revision: D14244362
fbshipit-source-id: 5f18100c08160e7514b3fd88ec47809cb37e9770
Summary:
Fixes T40544233
Exceptions should be thrown by value, not as a pointer, or else catching by reference (the recommended practice) won't work.
Reviewed By: priteshrnandgaonkar
Differential Revision: D14243337
fbshipit-source-id: 28677d7f1b3c025b67fb945f30116f3b751a89a8
Summary: Allows plugin to check if something is supported by that app, for example an extension command or a new feature, before trying to use it.
Reviewed By: passy
Differential Revision: D14225957
fbshipit-source-id: 3c5a29a06b56fe5f1da772824232547447872344
Summary:
I have released pods for Folly and RSocket on cocoapods. Thus we won't need to host different versions of these pods on our repository. I haven't removed those pods from `Specs` folder, as the current users of `0.16.2` would still be referring the Specs folder on the master branch. I will remove the specs folder once, all our pods are on cocoapods and we no longer need to host any pods.
Pull Request resolved: https://github.com/facebook/flipper/pull/379
Reviewed By: jknoxville
Differential Revision: D14185931
Pulled By: priteshrnandgaonkar
fbshipit-source-id: ea285024123e41c2d110827e26f79a72ef22c008
Summary: This info is still available in the diagnostic screen.
Reviewed By: passy
Differential Revision: D14164791
fbshipit-source-id: 1fc91da0b7437d530bddee6cbb5ea8c2d11f7b80
Summary: This release targets the support library and target SDK v28.
Reviewed By: muraziz
Differential Revision: D14151636
fbshipit-source-id: 70f4412dc0ccba3c77dda74dcf9b143782b90713
Summary: I got tripped up again with a physical iOS device that had the wrong date and time. SSL certs won't be valid. Adds a reminder to the diagnostic screen for anyone running into this in future.
Reviewed By: passy
Differential Revision: D14132411
fbshipit-source-id: 623120ef5252eae3e2fe0f6f653074f586f17e0e
Summary: The CSR has no need to change, don't recreate it every time we need it. Useful for the physical iOS case especially, because it connects to the portforwarder and keeps attempting to send a CSR.
Reviewed By: passy
Differential Revision: D14131617
fbshipit-source-id: 82a69d5aff813d77fa05dd13fc2441b784766d99
Summary:
We've seen some crashes from toRSocketPayload, but aren't sure of the cause.
When we fail the DCHECK, there won't be any log output, so log before it if it's going to fail.
Reviewed By: passy
Differential Revision: D14131409
fbshipit-source-id: b71e043f0b5feac3d2e44a34cc38c72086f966ad
Summary: Destructors should never throw, because if they are destructed while an exception is unwinding the stack the program will terminate.
Reviewed By: passy
Differential Revision: D14124423
fbshipit-source-id: 4b3d21663d44160bf24adacca25f50522d952e7c
Summary: Exceptions should never be thrown from destructors, because if an exception is already propogating at the time the destructor is called. Program will terminate.
Reviewed By: passy
Differential Revision: D14123526
fbshipit-source-id: 15ff74f4f14eb28a586055e1b1a5d54231b549da
