Skip to content

Commit 1ad5ace

Browse files
dannysufacebook-github-bot
authored andcommitted
Stop storing title in CDPHandler [1/2]
Summary: The `title` field is something that React Native's ConnectionDemux stores in CDPHandler. However, the data is not useful inside CDPHandler so it should be removed. Reviewed By: mattbfb Differential Revision: D50954257 fbshipit-source-id: ee590b8347981bf2fa0ad9ec3d824d7a953cf613
1 parent f0aa9c9 commit 1ad5ace

File tree

4 files changed

+18
-21
lines changed

4 files changed

+18
-21
lines changed

API/hermes/inspector/chrome/CDPHandler.cpp

+12-19
Original file line numberDiff line numberDiff line change
@@ -105,14 +105,9 @@ class CDPHandlerImpl : public message::RequestHandler,
105105
public debugger::EventObserver,
106106
public std::enable_shared_from_this<CDPHandlerImpl> {
107107
public:
108-
CDPHandlerImpl(
109-
std::unique_ptr<RuntimeAdapter> adapter,
110-
const std::string &title,
111-
bool waitForDebugger);
108+
CDPHandlerImpl(std::unique_ptr<RuntimeAdapter> adapter, bool waitForDebugger);
112109
~CDPHandlerImpl() override;
113110

114-
std::string getTitle() const;
115-
116111
bool registerCallbacks(
117112
CDPMessageCallbackFunction msgCallback,
118113
OnUnregisterFunction onUnregister);
@@ -336,7 +331,6 @@ class CDPHandlerImpl : public message::RequestHandler,
336331
/// inside the CDP Handler without requiring \p RuntimeAdapter::getRuntime
337332
/// to support use from arbitrary threads.
338333
HermesRuntime &runtime_;
339-
const std::string title_;
340334

341335
// preparedScripts_ stores user-entered scripts that have been prepared for
342336
// execution, and may be invoked by a later command.
@@ -428,11 +422,9 @@ class CDPHandlerImpl : public message::RequestHandler,
428422

429423
CDPHandlerImpl::CDPHandlerImpl(
430424
std::unique_ptr<RuntimeAdapter> adapter,
431-
const std::string &title,
432425
bool waitForDebugger)
433426
: runtimeAdapter_(std::move(adapter)),
434427
runtime_(runtimeAdapter_->getRuntime()),
435-
title_(title),
436428
awaitingDebuggerOnStart_(waitForDebugger) {
437429
// Install __tickleJs. Do this activity before the call to setEventObserver,
438430
// so we don't get any didPause callback firings for these.
@@ -456,13 +448,6 @@ CDPHandlerImpl::~CDPHandlerImpl() {
456448
// by other mutex
457449
}
458450

459-
std::string CDPHandlerImpl::getTitle() const {
460-
// This is a public function, but the mutex is not required
461-
// as we're just returning member that is unchanged for the
462-
// lifetime of this instance.
463-
return title_;
464-
}
465-
466451
bool CDPHandlerImpl::registerCallbacks(
467452
CDPMessageCallbackFunction msgCallback,
468453
OnUnregisterFunction onUnregister) {
@@ -1639,6 +1624,14 @@ bool CDPHandlerImpl::validateExecutionContext(
16391624
/*
16401625
* CDPHandler
16411626
*/
1627+
std::shared_ptr<CDPHandler> CDPHandler::create(
1628+
std::unique_ptr<RuntimeAdapter> adapter,
1629+
bool waitForDebugger) {
1630+
// Can't use make_shared here since the constructor is private.
1631+
return std::shared_ptr<CDPHandler>(
1632+
new CDPHandler(std::move(adapter), "", waitForDebugger));
1633+
}
1634+
16421635
std::shared_ptr<CDPHandler> CDPHandler::create(
16431636
std::unique_ptr<RuntimeAdapter> adapter,
16441637
const std::string &title,
@@ -1654,15 +1647,15 @@ CDPHandler::CDPHandler(
16541647
bool waitForDebugger)
16551648
: impl_(std::make_shared<CDPHandlerImpl>(
16561649
std::move(adapter),
1657-
title,
1658-
waitForDebugger)) {
1650+
waitForDebugger)),
1651+
title_(title) {
16591652
impl_->installLogHandler();
16601653
}
16611654

16621655
CDPHandler::~CDPHandler() = default;
16631656

16641657
std::string CDPHandler::getTitle() const {
1665-
return impl_->getTitle();
1658+
return title_;
16661659
}
16671660

16681661
bool CDPHandler::registerCallbacks(

API/hermes/inspector/chrome/CDPHandler.h

+5
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ class INSPECTOR_EXPORT CDPHandler {
5353
/// should generally called before you start running any JS in the runtime.
5454
/// This should also be called on the runtime thread, as methods are invoked
5555
/// on the given \p adapter.
56+
static std::shared_ptr<CDPHandler> create(
57+
std::unique_ptr<RuntimeAdapter> adapter,
58+
bool waitForDebugger = false);
59+
/// Temporarily kept to allow React Native build to still work
5660
static std::shared_ptr<CDPHandler> create(
5761
std::unique_ptr<RuntimeAdapter> adapter,
5862
const std::string &title,
@@ -84,6 +88,7 @@ class INSPECTOR_EXPORT CDPHandler {
8488

8589
private:
8690
std::shared_ptr<CDPHandlerImpl> impl_;
91+
const std::string title_;
8792
};
8893

8994
} // namespace chrome

API/hermes/inspector/chrome/cli/main.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ static void runScript(const std::string &scriptSource, const std::string &url) {
132132
std::make_unique<fbhermes::inspector_modern::SharedRuntimeAdapter>(
133133
runtime);
134134
std::shared_ptr<CDPHandler> cdpHandler =
135-
CDPHandler::create(std::move(adapter), "hermes-chrome-debug-server");
135+
CDPHandler::create(std::move(adapter));
136136
std::thread debuggerLoop(runDebuggerLoop, std::ref(cdpHandler), scriptSource);
137137

138138
fbhermes::HermesRuntime::DebugFlags flags{};

API/hermes/inspector/chrome/tests/SyncConnection.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ SyncConnection::SyncConnection(
3535
bool waitForDebugger)
3636
: cdpHandler_(CDPHandler::create(
3737
std::make_unique<ExecutorRuntimeAdapter>(runtime),
38-
"testConn",
3938
waitForDebugger)) {
4039
registerCallbacks();
4140
}

0 commit comments

Comments
 (0)