diff --git a/desktop/app/src/Client.tsx b/desktop/app/src/Client.tsx index 24b4f0f6d..dd07950e5 100644 --- a/desktop/app/src/Client.tsx +++ b/desktop/app/src/Client.tsx @@ -114,6 +114,7 @@ export default class Client extends EventEmitter { sdkVersion: number; messageIdCounter: number; plugins: Plugins; + backgroundPlugins: Plugins; connection: FlipperClientConnection | null | undefined; store: Store; activePlugins: Set; @@ -145,6 +146,7 @@ export default class Client extends EventEmitter { super(); this.connected = true; this.plugins = plugins ? plugins : []; + this.backgroundPlugins = []; this.connection = conn; this.id = id; this.query = query; @@ -235,13 +237,27 @@ export default class Client extends EventEmitter { return this.plugins.includes(Plugin.id); } + isBackgroundPlugin(pluginId: string) { + return this.backgroundPlugins.includes(pluginId); + } + async init() { this.setMatchingDevice(); - await this.getPlugins(); + await this.loadPlugins(); + this.backgroundPlugins = await this.getBackgroundPlugins(); + this.backgroundPlugins.forEach((plugin) => { + if ( + this.store + .getState() + .connections.userStarredPlugins[this.query.app]?.includes(plugin) + ) { + this.initPlugin(plugin); + } + }); } // get the supported plugins - async getPlugins(): Promise { + async loadPlugins(): Promise { const plugins = await this.rawCall<{plugins: Plugins}>( 'getPlugins', false, @@ -265,9 +281,44 @@ export default class Client extends EventEmitter { return plugins; } + // get the supported background plugins + async getBackgroundPlugins(): Promise { + if (this.sdkVersion < 4) { + return []; + } + return await this.rawCall<{plugins: Plugins}>( + 'getBackgroundPlugins', + false, + ).then((data) => data.plugins); + } + // get the plugins, and update the UI async refreshPlugins() { - await this.getPlugins(); + const oldBackgroundPlugins = this.backgroundPlugins; + await this.loadPlugins(); + const newBackgroundPlugins = await this.getBackgroundPlugins(); + this.backgroundPlugins = newBackgroundPlugins; + // diff the background plugin list, disconnect old, connect new ones + oldBackgroundPlugins.forEach((plugin) => { + if ( + !newBackgroundPlugins.includes(plugin) && + this.store + .getState() + .connections.userStarredPlugins[this.query.app]?.includes(plugin) + ) { + this.deinitPlugin(plugin); + } + }); + newBackgroundPlugins.forEach((plugin) => { + if ( + !oldBackgroundPlugins.includes(plugin) && + this.store + .getState() + .connections.userStarredPlugins[this.query.app]?.includes(plugin) + ) { + this.initPlugin(plugin); + } + }); this.emit('plugins-change'); } diff --git a/desktop/app/src/plugin.tsx b/desktop/app/src/plugin.tsx index 1cb12ea0a..ebd240dc8 100644 --- a/desktop/app/src/plugin.tsx +++ b/desktop/app/src/plugin.tsx @@ -269,18 +269,25 @@ export class FlipperPlugin< _teardown() { // automatically unsubscribe subscriptions + const pluginId = this.constructor.id; for (const {method, callback} of this.subscriptions) { - this.realClient.unsubscribe(this.constructor.id, method, callback); + this.realClient.unsubscribe(pluginId, method, callback); } // run plugin teardown this.teardown(); - if (this.realClient.connected) { - this.realClient.deinitPlugin(this.constructor.id); + if ( + this.realClient.connected && + !this.realClient.isBackgroundPlugin(pluginId) + ) { + this.realClient.deinitPlugin(pluginId); } } _init() { - this.realClient.initPlugin(this.constructor.id); + const pluginId = this.constructor.id; + if (!this.realClient.isBackgroundPlugin(pluginId)) { + this.realClient.initPlugin(pluginId); + } this.init(); } } diff --git a/desktop/app/src/reducers/connections.tsx b/desktop/app/src/reducers/connections.tsx index 1f10ca7ce..876339342 100644 --- a/desktop/app/src/reducers/connections.tsx +++ b/desktop/app/src/reducers/connections.tsx @@ -277,6 +277,9 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => { case 'STAR_PLUGIN': { const {selectedPlugin, selectedApp} = action.payload; + const client = state.clients.find( + (client) => client.query.app === selectedApp, + ); return produce(state, (draft) => { if (!draft.userStarredPlugins[selectedApp]) { draft.userStarredPlugins[selectedApp] = [selectedPlugin]; @@ -285,8 +288,14 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => { const idx = plugins.indexOf(selectedPlugin); if (idx === -1) { plugins.push(selectedPlugin); + if (client?.isBackgroundPlugin(selectedPlugin)) { + client.initPlugin(selectedPlugin); + } } else { plugins.splice(idx, 1); + if (client?.isBackgroundPlugin(selectedPlugin)) { + client.deinitPlugin(selectedPlugin); + } } } }); diff --git a/desktop/app/src/utils/messageQueue.tsx b/desktop/app/src/utils/messageQueue.tsx index 0ec940bb7..1c7e4f3a1 100644 --- a/desktop/app/src/utils/messageQueue.tsx +++ b/desktop/app/src/utils/messageQueue.tsx @@ -221,7 +221,11 @@ export function processMessageLater( ), ); break; - // In all other cases, messages will be dropped... + default: + // In all other cases, messages will be dropped... + console.warn( + `Received message for disabled plugin ${plugin.id}: ${message.method}, dropping..`, + ); } } diff --git a/docs/extending/new-clients.mdx b/docs/extending/new-clients.mdx index e30545ea3..d482c787a 100644 --- a/docs/extending/new-clients.mdx +++ b/docs/extending/new-clients.mdx @@ -70,6 +70,23 @@ Response = { } ``` + +### getBackgroundPlugins + +Returns a subset of the available plugins returned by `getPlugin`. The background connections will automatically receive a connection from Flipper once it starts (and if the plugins are enabled), rather than waiting for the user to open the plugin. + +``` +Request = { + "method": "getBackgroundPlugins", +} + +Response = { + "success": { + "plugins": Array + }, +} +``` + ### init Initialize a plugin. This should result in an onConnected call on the appropriate plugin. Plugins should by nature be lazy and should not be initialized up front as this may incur significant cost. The Flipper desktop client knows when a plugin is needed and should control when to initialize them. ``` diff --git a/iOS/FlipperKitTests/FlipperClientTests.mm b/iOS/FlipperKitTests/FlipperClientTests.mm index c87129891..45cdc1004 100644 --- a/iOS/FlipperKitTests/FlipperClientTests.mm +++ b/iOS/FlipperKitTests/FlipperClientTests.mm @@ -92,7 +92,7 @@ FlipperClient* objcClient; XCTAssertEqual(successes[0], expected); } -- (void)testPluginActivatedInBackgroundMode { +- (void)testPluginNotActivatedInBackgroundMode { __block BOOL pluginConnected = NO; BlockBasedSonarPlugin* cat = [[BlockBasedSonarPlugin alloc] initIdentifier:@"cat" @@ -106,7 +106,7 @@ FlipperClient* objcClient; [objcClient addPlugin:cat]; [objcClient start]; - XCTAssertTrue(pluginConnected); + XCTAssertFalse(pluginConnected); } - (void)testPluginNotActivatedInNonBackgroundMode { @@ -153,7 +153,7 @@ FlipperClient* objcClient; } - (void)testConnectAndDisconnectCallbackForBackgroundCase { - __block BOOL pluginConnected = YES; + __block BOOL pluginConnected = NO; BlockBasedSonarPlugin* cat = [[BlockBasedSonarPlugin alloc] initIdentifier:@"cat" connect:^(id) { @@ -166,7 +166,27 @@ FlipperClient* objcClient; [objcClient addPlugin:cat]; [objcClient start]; + XCTAssertFalse(pluginConnected); + + folly::dynamic messageInit = folly::dynamic::object("method", "init")( + "params", folly::dynamic::object("plugin", "cat")); + std::unique_ptr responder = + std::make_unique(); + socket->callbacks->onMessageReceived(messageInit, std::move(responder)); XCTAssertTrue(pluginConnected); + + folly::dynamic messageDeInit = folly::dynamic::object("method", "deinit")( + "params", folly::dynamic::object("plugin", "cat")); + std::unique_ptr responder2 = + std::make_unique(); + socket->callbacks->onMessageReceived(messageDeInit, std::move(responder2)); + XCTAssertFalse(pluginConnected); + + std::unique_ptr responder3 = + std::make_unique(); + socket->callbacks->onMessageReceived(messageInit, std::move(responder3)); + XCTAssertTrue(pluginConnected); + [objcClient stop]; XCTAssertFalse(pluginConnected); } @@ -184,10 +204,16 @@ FlipperClient* objcClient; runInBackground:YES]; [objcClient addPlugin:cat]; - // Since background plugin's didconnect is called as soon as flipper client - // starts XCTAssertNoThrow([objcClient start]); - XCTAssertTrue(pluginConnected); // To be sure that connect block is called + XCTAssertFalse(pluginConnected); + + folly::dynamic messageInit = folly::dynamic::object("method", "init")( + "params", folly::dynamic::object("plugin", "cat")); + std::unique_ptr responder = + std::make_unique(); + XCTAssertNoThrow( + socket->callbacks->onMessageReceived(messageInit, std::move(responder))); + XCTAssertTrue(pluginConnected); } - (void)testCrashSuppressionInDisconnectCallback { @@ -205,6 +231,12 @@ FlipperClient* objcClient; [objcClient addPlugin:cat]; [objcClient start]; + folly::dynamic messageInit = folly::dynamic::object("method", "init")( + "params", folly::dynamic::object("plugin", "cat")); + std::unique_ptr responder = + std::make_unique(); + socket->callbacks->onMessageReceived(messageInit, std::move(responder)); + XCTAssertNoThrow( [objcClient stop]); // Stopping client will call disconnect of the plugin XCTAssertTrue(isCalled); // To be sure that connect block is called @@ -264,13 +296,19 @@ FlipperClient* objcClient; [objcClient addPlugin:cat]; [objcClient start]; + folly::dynamic messageInit = folly::dynamic::object("method", "init")( + "params", folly::dynamic::object("plugin", "PluginIdentifier")); + std::unique_ptr responder = + std::make_unique(); + socket->callbacks->onMessageReceived(messageInit, std::move(responder)); + folly::dynamic message = folly::dynamic::object("id", 1)("method", "execute")( "params", folly::dynamic::object("api", "PluginIdentifier")( "method", "MethodName")); - std::unique_ptr responder = + std::unique_ptr responder2 = std::make_unique(); - socket->callbacks->onMessageReceived(message, std::move(responder)); + socket->callbacks->onMessageReceived(message, std::move(responder2)); XCTAssertTrue(isCalled); } @@ -296,18 +334,24 @@ FlipperClient* objcClient; [objcClient addPlugin:cat]; [objcClient start]; + folly::dynamic messageInit = folly::dynamic::object("method", "init")( + "params", folly::dynamic::object("plugin", "PluginIdentifier")); + std::unique_ptr responder = + std::make_unique(); + socket->callbacks->onMessageReceived(messageInit, std::move(responder)); + folly::dynamic message = folly::dynamic::object("id", 1)("method", "execute")( "params", folly::dynamic::object("api", "PluginIdentifier")( "method", "MethodName")); std::vector successes = std::vector(); std::vector errors = std::vector(); - std::unique_ptr responder = + std::unique_ptr responder2 = std::make_unique( &successes, &errors); XCTAssertNoThrow( - socket->callbacks->onMessageReceived(message, std::move(responder))); + socket->callbacks->onMessageReceived(message, std::move(responder2))); XCTAssertTrue(isCalled); XCTAssertEqual(successes.size(), 0); XCTAssertEqual(errors.size(), 1); diff --git a/xplat/Flipper/FlipperClient.cpp b/xplat/Flipper/FlipperClient.cpp index dc3325e54..6b897d581 100644 --- a/xplat/Flipper/FlipperClient.cpp +++ b/xplat/Flipper/FlipperClient.cpp @@ -69,12 +69,6 @@ void FlipperClient::addPlugin(std::shared_ptr plugin) { step->complete(); if (connected_) { refreshPlugins(); - if (plugin->runInBackground()) { - auto& conn = connections_[plugin->identifier()]; - conn = std::make_shared( - socket_.get(), plugin->identifier()); - plugin->didConnect(conn); - } } }); } @@ -95,27 +89,6 @@ void FlipperClient::removePlugin(std::shared_ptr plugin) { }); } -void FlipperClient::startBackgroundPlugins() { - std::cout << "Activating Background Plugins..." << std::endl; - for (std::map>::iterator it = - plugins_.begin(); - it != plugins_.end(); - ++it) { - std::cout << it->first << std::endl; - if (it->second.get()->runInBackground()) { - try { - auto& conn = connections_[it->first]; - conn = - std::make_shared(socket_.get(), it->first); - it->second.get()->didConnect(conn); - } catch (std::exception& e) { - log("Exception starting background plugin: " + it->first + ". " + - e.what()); - } - } - } -} - std::shared_ptr FlipperClient::getPlugin( const std::string& identifier) { std::lock_guard lock(mutex_); @@ -130,6 +103,15 @@ bool FlipperClient::hasPlugin(const std::string& identifier) { return plugins_.find(identifier) != plugins_.end(); } +void FlipperClient::connect(std::shared_ptr plugin) { + if (connections_.find(plugin->identifier()) == connections_.end()) { + auto& conn = connections_[plugin->identifier()]; + conn = std::make_shared( + socket_.get(), plugin->identifier()); + plugin->didConnect(conn); + } +} + void FlipperClient::disconnect(std::shared_ptr plugin) { const auto& conn = connections_.find(plugin->identifier()); if (conn != connections_.end()) { @@ -151,7 +133,6 @@ void FlipperClient::onConnected() { std::lock_guard lock(mutex_); connected_ = true; - startBackgroundPlugins(); }); } @@ -189,6 +170,18 @@ void FlipperClient::onMessageReceived( return; } + if (method == "getBackgroundPlugins") { + dynamic identifiers = dynamic::array(); + for (const auto& elem : plugins_) { + if (elem.second->runInBackground()) { + identifiers.push_back(elem.first); + } + } + dynamic response = dynamic::object("plugins", identifiers); + responder->success(response); + return; + } + if (method == "init") { const auto identifier = params["plugin"].getString(); if (plugins_.find(identifier) == plugins_.end()) { @@ -200,12 +193,7 @@ void FlipperClient::onMessageReceived( return; } const auto plugin = plugins_.at(identifier); - if (!plugin.get()->runInBackground()) { - auto& conn = connections_[plugin->identifier()]; - conn = std::make_shared( - socket_.get(), plugin->identifier()); - plugin->didConnect(conn); - } + connect(plugin); return; } @@ -220,9 +208,7 @@ void FlipperClient::onMessageReceived( return; } const auto plugin = plugins_.at(identifier); - if (!plugin.get()->runInBackground()) { - disconnect(plugin); - } + disconnect(plugin); return; } diff --git a/xplat/Flipper/FlipperClient.h b/xplat/Flipper/FlipperClient.h index 1c107d6a6..28071e818 100644 --- a/xplat/Flipper/FlipperClient.h +++ b/xplat/Flipper/FlipperClient.h @@ -108,8 +108,8 @@ class FlipperClient : public FlipperConnectionManager::Callbacks { std::mutex mutex_; std::shared_ptr flipperState_; + void connect(std::shared_ptr plugin); void disconnect(std::shared_ptr plugin); - void startBackgroundPlugins(); std::string callstack(); void handleError(std::exception& e); }; diff --git a/xplat/Flipper/FlipperConnectionManagerImpl.cpp b/xplat/Flipper/FlipperConnectionManagerImpl.cpp index 288e9825a..c10fab364 100644 --- a/xplat/Flipper/FlipperConnectionManagerImpl.cpp +++ b/xplat/Flipper/FlipperConnectionManagerImpl.cpp @@ -35,7 +35,7 @@ static constexpr int maxPayloadSize = 0xFFFFFF; // Not a public-facing version number. // Used for compatibility checking with desktop flipper. // To be bumped for every core platform interface change. -static constexpr int sdkVersion = 3; +static constexpr int sdkVersion = 4; namespace facebook { namespace flipper { diff --git a/xplat/FlipperTests/FlipperClientTests.cpp b/xplat/FlipperTests/FlipperClientTests.cpp index e94db2388..c855751d1 100644 --- a/xplat/FlipperTests/FlipperClientTests.cpp +++ b/xplat/FlipperTests/FlipperClientTests.cpp @@ -312,6 +312,10 @@ TEST_F(FlipperClientTest, testExceptionUnknownApi) { } TEST_F(FlipperClientTest, testBackgroundPluginActivated) { + dynamic messageInit = dynamic::object("method", "init")( + "params", dynamic::object("plugin", "Test")); + dynamic messageDeinit = dynamic::object("method", "deinit")( + "params", dynamic::object("plugin", "Test")); bool pluginConnected = false; const auto connectionCallback = [&](std::shared_ptr conn) { pluginConnected = true; @@ -322,7 +326,26 @@ TEST_F(FlipperClientTest, testBackgroundPluginActivated) { client->addPlugin(plugin); client->start(); - EXPECT_TRUE(pluginConnected); + EXPECT_FALSE(pluginConnected); + + { + auto responder = std::make_unique(); + socket->onMessageReceived(messageInit, getResponder()); + EXPECT_TRUE(pluginConnected); + } + + { + auto responder = std::make_unique(); + socket->onMessageReceived(messageDeinit, getResponder()); + EXPECT_FALSE(pluginConnected); + } + + { + auto responder = std::make_unique(); + socket->onMessageReceived(messageInit, getResponder()); + EXPECT_TRUE(pluginConnected); + } + client->stop(); EXPECT_FALSE(pluginConnected); }