Debugger outstanding issues (#15392)

* move string_format to  Microsoft::React namespace and

* Provide the real process ID to the RuntimeSamplingProfile constructor

* remove DevSupportManager::OpenDevTools not needed with  modern inspector infra

* yarn lint:format and fix

* Change files

* change files

* Revert "change files"

This reverts commit d8b0b38.

* Change files

* improve string_format implementation

* remove old sampling profiler

* revert IReactDispatcher

* resolve nit comments

Author: iamAbhi-916

change/react-native-windows-f1eeb076-2cad-486c-88ba-7d9f68b8fc3a.json

@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "fix outstanding modern debugger issues",
+  "packageName": "react-native-windows",
+  "email": "74712637+iamAbhi-916@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}

vnext/Desktop.UnitTests/UtilsTest.cpp

@@ -268,6 +268,85 @@ TEST_CLASS(UtilsTest)
   }
 
 #pragma endregion Base64 Tests
+
+#pragma region FormatString Tests
+
+  TEST_METHOD(UtilsTest_StringFormat_Simple)
+  {
+    std::string result = Microsoft::React::FormatString("Hello, %s!", "World");
+    Assert::AreEqual("Hello, World!", result.c_str());
+  }
+
+  TEST_METHOD(UtilsTest_StringFormat_Integer)
+  {
+    std::string result = Microsoft::React::FormatString("Port: %d", 8081);
+    Assert::AreEqual("Port: 8081", result.c_str());
+  }
+
+  TEST_METHOD(UtilsTest_StringFormat_Multiple)
+  {
+    std::string result = Microsoft::React::FormatString("%s:%d", "localhost", 8081);
+    Assert::AreEqual("localhost:8081", result.c_str());
+  }
+
+  TEST_METHOD(UtilsTest_StringFormat_Complex)
+  {
+    std::string result = Microsoft::React::FormatString(
+      "http://%s/%s.bundle?platform=%s&dev=%s&hot=%s",
+      "localhost:8081",
+      "index",
+      "windows",
+      "true",
+      "false");
+    Assert::AreEqual(
+      "http://localhost:8081/index.bundle?platform=windows&dev=true&hot=false",
+      result.c_str());
+  }
+
+  TEST_METHOD(UtilsTest_StringFormat_EmptyString)
+  {
+    std::string result = Microsoft::React::FormatString("");
+    Assert::AreEqual("", result.c_str());
+  }
+
+  TEST_METHOD(UtilsTest_StringFormat_NullPtr)
+  {
+    std::string result = Microsoft::React::FormatString(nullptr);
+    Assert::AreEqual("", result.c_str());
+  }
+
+  TEST_METHOD(UtilsTest_StringFormat_NoArgs)
+  {
+    std::string result = Microsoft::React::FormatString("no args here");
+    Assert::AreEqual("no args here", result.c_str());
+  }
+
+  TEST_METHOD(UtilsTest_StringFormat_LargeString)
+  {
+    std::string longString(1000, 'a');
+    std::string result = Microsoft::React::FormatString("%s", longString.c_str());
+    Assert::AreEqual(longString.c_str(), result.c_str());
+  }
+
+  TEST_METHOD(UtilsTest_StringFormat_MixedTypes)
+  {
+    std::string result = Microsoft::React::FormatString(
+      "Int: %d, Uint: %u, Hex: %x, String: %s, Float: %.2f",
+      -42,
+      42u,
+      255,
+      "test",
+      3.14159);
+    Assert::AreEqual("Int: -42, Uint: 42, Hex: ff, String: test, Float: 3.14", result.c_str());
+  }
+
+  TEST_METHOD(UtilsTest_StringFormat_SpecialChars)
+  {
+    std::string result = Microsoft::React::FormatString("100%% complete");
+    Assert::AreEqual("100% complete", result.c_str());
+  }
+
+#pragma endregion FormatString Tests
 };
 
 // clang-format on

vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp

@@ -177,7 +177,6 @@ ReactInstanceWin::ReactInstanceWin(
                                                                propBag = m_options.Properties,
                                                                reactContext = m_reactContext]() noexcept {
         whenLoaded.TryCancel(); // It only has an effect if whenLoaded was not set before
-        Microsoft::ReactNative::HermesRuntimeHolder::storeTo(ReactPropertyBag(reactContext->Properties()), nullptr);
         if (onDestroyed) {
           onDestroyed.Get()->Invoke(reactContext);
         }

vnext/Microsoft.ReactNative/Views/DevMenu.cpp

@@ -6,7 +6,6 @@
 #include "DevMenu.h"
 
 #include <winrt/Windows.ApplicationModel.DataTransfer.h>
-#include "Hermes/HermesSamplingProfiler.h"
 #include "IReactDispatcher.h"
 #include "Modules/DevSettingsModule.h"
 
@@ -68,30 +67,11 @@ void ToggleFastRefresh(Mso::CntPtr<Mso::React::IReactContext> const &reactContex
   DevSettings::Reload(React::ReactPropertyBag(reactContext->Properties()));
 }
 
-winrt::fire_and_forget ToggleHermesProfiler(Mso::CntPtr<Mso::React::IReactContext> const &reactContext) noexcept {
-  if (!Microsoft::ReactNative::HermesSamplingProfiler::IsStarted()) {
-    Microsoft::ReactNative::HermesSamplingProfiler::Start(reactContext);
-  } else {
-    auto traceFilePath = co_await Microsoft::ReactNative::HermesSamplingProfiler::Stop(reactContext);
-    auto uiDispatcher = React::implementation::ReactDispatcher::GetUIDispatcher(reactContext->Properties());
-    uiDispatcher.Post([traceFilePath]() {
-      DataTransfer::DataPackage data;
-      data.SetText(winrt::to_hstring(traceFilePath));
-      DataTransfer::Clipboard::SetContentWithOptions(data, nullptr);
-    });
-  }
-}
-
 const wchar_t *FastRefreshLabel(Mso::CntPtr<Mso::React::IReactContext> const &reactContext) noexcept {
   return Mso::React::ReactOptions::UseFastRefresh(reactContext->Properties()) ? L"Disable Fast Refresh"
                                                                               : L"Enable Fast Refresh";
 }
 
-const wchar_t *HermesProfilerLabel(Mso::CntPtr<Mso::React::IReactContext> const &reactContext) noexcept {
-  return !Microsoft::ReactNative::HermesSamplingProfiler::IsStarted() ? L"Start Hermes sampling profiler"
-                                                                      : L"Stop and copy trace path to clipboard";
-}
-
 struct WindowsPopupMenuDevMenu : public IDevMenu, public std::enable_shared_from_this<WindowsPopupMenuDevMenu> {
   WindowsPopupMenuDevMenu(Mso::CntPtr<Mso::React::IReactContext> const &reactContext) : m_context(reactContext) {}
 
@@ -127,12 +107,6 @@ struct WindowsPopupMenuDevMenu : public IDevMenu, public std::enable_shared_from
           DevSettings::ToggleElementInspector(*context);
         }));
 
-    if (Mso::React::ReactOptions::JsiEngine(m_context->Properties()) == Mso::React::JSIEngine::Hermes) {
-      m_menu.Commands().Append(winrt::Windows::UI::Popups::UICommand(
-          HermesProfilerLabel(m_context),
-          [context = m_context](winrt::Windows::UI::Popups::IUICommand const &) { ToggleHermesProfiler(context); }));
-    }
-
     m_menu.ShowAsync({0, 0});
   }
 

vnext/Shared/DevServerHelper.h

@@ -4,6 +4,7 @@
 #pragma once
 
 #include <cxxreact/JSExecutor.h>
+#include "Utils.h"
 
 namespace facebook {
 namespace react {
@@ -13,13 +14,14 @@ class DevServerHelper {
   DevServerHelper() = default;
 
   static std::string get_WebsocketProxyUrl(const std::string &sourceBundleHost, const uint16_t sourceBundlePort) {
-    return string_format(WebsocketProxyUrlFormat, GetDeviceLocalHost(sourceBundleHost, sourceBundlePort).c_str());
+    return Microsoft::React::FormatString(
+        WebsocketProxyUrlFormat, GetDeviceLocalHost(sourceBundleHost, sourceBundlePort).c_str());
   }
 
   static std::string get_LaunchDevToolsCommandUrl(
       const std::string &sourceBundleHost,
       const uint16_t sourceBundlePort) {
-    return string_format(
+    return Microsoft::React::FormatString(
         LaunchDevToolsCommandUrlFormat, GetDeviceLocalHost(sourceBundleHost, sourceBundlePort).c_str());
   }
 
@@ -36,16 +38,17 @@ class DevServerHelper {
     std::string hermesBytecodeVersionQuery;
     if (hermesBytecodeVersion > 0) {
       static constexpr const char HermesBytecodeVersionQueryFormat[] = "&runtimeBytecodeVersion=%d";
-      hermesBytecodeVersionQuery = string_format(HermesBytecodeVersionQueryFormat, hermesBytecodeVersion);
+      hermesBytecodeVersionQuery =
+          Microsoft::React::FormatString(HermesBytecodeVersionQueryFormat, hermesBytecodeVersion);
     }
 
     std::string appIdQuery;
     if (bundleAppId.size() > 0) {
       static constexpr const char AppIdQueryFormat[] = "&app=%s";
-      appIdQuery = string_format(AppIdQueryFormat, bundleAppId.c_str());
+      appIdQuery = Microsoft::React::FormatString(AppIdQueryFormat, bundleAppId.c_str());
     }
 
-    return string_format(
+    return Microsoft::React::FormatString(
         BundleUrlFormat,
         GetDeviceLocalHost(sourceBundleHost, sourceBundlePort).c_str(),
         jsbundle.c_str(),
@@ -58,17 +61,19 @@ class DevServerHelper {
   }
 
   static std::string get_OnChangeEndpointUrl(const std::string &sourceBundleHost, const uint16_t sourceBundlePort) {
-    return string_format(OnChangeEndpointUrlFormat, GetDeviceLocalHost(sourceBundleHost, sourceBundlePort).c_str());
+    return Microsoft::React::FormatString(
+        OnChangeEndpointUrlFormat, GetDeviceLocalHost(sourceBundleHost, sourceBundlePort).c_str());
   }
 
   static std::string get_PackagerConnectionUrl(const std::string &sourceBundleHost, const uint16_t sourceBundlePort) {
-    return string_format(PackagerConnectionUrlFormat, GetDeviceLocalHost(sourceBundleHost, sourceBundlePort).c_str());
+    return Microsoft::React::FormatString(
+        PackagerConnectionUrlFormat, GetDeviceLocalHost(sourceBundleHost, sourceBundlePort).c_str());
   }
 
   static std::string get_PackagerOpenStackFrameUrl(
       const std::string &sourceBundleHost,
       const uint16_t sourceBundlePort) {
-    return string_format(
+    return Microsoft::React::FormatString(
         PackagerOpenStackFrameUrlFormat, GetDeviceLocalHost(sourceBundleHost, sourceBundlePort).c_str());
   }
 
@@ -78,7 +83,7 @@ class DevServerHelper {
       const std::string &deviceName,
       const std::string &packageName,
       const std::string &deviceId) {
-    return string_format(
+    return Microsoft::React::FormatString(
         InspectorDeviceUrlFormat,
         GetDeviceLocalHost(packagerHost, packagerPort).c_str(),
         deviceName.c_str(),
@@ -88,7 +93,7 @@ class DevServerHelper {
 
   static std::string
   get_OpenDebuggerUrl(const std::string &packagerHost, const uint16_t packagerPort, const std::string &deviceId) {
-    return string_format(
+    return Microsoft::React::FormatString(
         OpenDebuggerUrlFormat, GetDeviceLocalHost(packagerHost, packagerPort).c_str(), deviceId.c_str());
   }
 
@@ -97,7 +102,7 @@ class DevServerHelper {
 
  private:
   static std::string GetDeviceLocalHost(const std::string &sourceBundleHost, const uint16_t sourceBundlePort) {
-    return string_format(
+    return Microsoft::React::FormatString(
         DeviceLocalHostFormat,
         sourceBundleHost.empty() ? DefaultPackagerHost : sourceBundleHost.c_str(),
         sourceBundlePort ? sourceBundlePort : DefaultPackagerPort);
@@ -118,15 +123,6 @@ class DevServerHelper {
 
   static constexpr const char PackagerOkStatus[] = "packager-status:running";
   const int LongPollFailureDelayMs = 5000;
-
-  // TODO: [vmoroz]  avoid using vaiadic args for the format and move it to a utility class.
-  template <typename... Args>
-  static std::string string_format(const char *format, Args... args) {
-    size_t size = snprintf(nullptr, 0, format, args...) + 1;
-    std::unique_ptr<char[]> buf(new char[size]);
-    snprintf(buf.get(), size, format, args...);
-    return std::string(buf.get(), buf.get() + size - 1);
-  }
 };
 
 } // namespace react

vnext/Shared/DevSupportManager.cpp

@@ -253,22 +253,6 @@ void DevSupportManager::StopPollingLiveReload() {
   m_cancellation_token = true;
 }
 
-// TODO: (vmoroz) Use or delete this function
-void DevSupportManager::OpenDevTools(const std::string &bundleAppId) {
-  winrt::Windows::Web::Http::Filters::HttpBaseProtocolFilter filter;
-  filter.CacheControl().ReadBehavior(winrt::Windows::Web::Http::Filters::HttpCacheReadBehavior::NoCache);
-  winrt::Windows::Web::Http::HttpClient httpClient(filter);
-  // TODO: Use currently configured dev server host
-  winrt::Windows::Foundation::Uri uri(
-      Microsoft::Common::Unicode::Utf8ToUtf16(facebook::react::DevServerHelper::get_OpenDebuggerUrl(
-          std::string{DevServerHelper::DefaultPackagerHost},
-          DevServerHelper::DefaultPackagerPort,
-          GetDeviceId(GetPackageName(bundleAppId)))));
-
-  winrt::Windows::Web::Http::HttpRequestMessage request(winrt::Windows::Web::Http::HttpMethod::Post(), uri);
-  httpClient.SendRequestAsync(request);
-}
-
 void DevSupportManager::EnsureInspectorPackagerConnection(
     [[maybe_unused]] const std::string &packagerHost,
     [[maybe_unused]] const uint16_t packagerPort,

vnext/Shared/DevSupportManager.h

@@ -45,7 +45,6 @@ class DevSupportManager final : public facebook::react::IDevSupportManager {
       const uint16_t sourceBundlePort,
       std::function<void()> onChangeCallback) override;
   virtual void StopPollingLiveReload() override;
-  virtual void OpenDevTools(const std::string &bundleAppId) override;
 
   virtual void EnsureInspectorPackagerConnection(
       const std::string &packagerHost,

vnext/Shared/Hermes/HermesRuntimeTargetDelegate.cpp

@@ -15,6 +15,7 @@
 
 #include "HermesRuntimeTargetDelegate.h"
 #include <jsinspector-modern/RuntimeTarget.h>
+#include <windows.h>
 #include <utility>
 #include "HermesRuntimeAgentDelegate.h"
 
@@ -268,7 +269,10 @@ tracing::RuntimeSamplingProfile HermesRuntimeTargetDelegate::collectSamplingProf
 
   // Return the complete profile with samples. Wrap the raw profile since it owns the strings.
   return tracing::RuntimeSamplingProfile(
-      "Hermes", 1234, std::move(readerState.samples), std::make_unique<HermesRawRuntimeProfile>(std::move(profile)));
+      "Hermes",
+      GetCurrentProcessId(),
+      std::move(readerState.samples),
+      std::make_unique<HermesRawRuntimeProfile>(std::move(profile)));
 }
 
 } // namespace Microsoft::ReactNative