Changeset 286579 in webkit

Timestamp:

Dec 6, 2021, 4:38:11 PM (5 years ago)

Author:

beidson@apple.com

Message:

webpushd/webpushtool debugging additions
​https://bugs.webkit.org/show_bug.cgi?id=233864

Reviewed by Alex Christensen.

Source/WebKit:

Covered by API tests.

This patch:

• Teaches webpushtool the ability to wait for a reconnect after losing its connection
• Starts actually broadcasting meaningful debug messages from webpushd

• webpushd/AppBundleRequest.mm:

(WebPushD::AppBundleRequest::start):
(WebPushD::AppBundlePermissionsRequest::didCheckForExistingBundle):
(WebPushD::AppBundlePermissionsRequest::didCreateAppBundle):
(WebPushD::AppBundleDeletionRequest::didDeleteExistingBundleWithError):

• webpushd/PushClientConnection.h:
• webpushd/PushClientConnection.mm:

(WebPushD::ClientConnection::setDebugModeIsEnabled):
(WebPushD::ClientConnection::broadcastDebugMessage):
(WebPushD::ClientConnection::connectionClosed):

• webpushd/WebPushDaemon.mm:

(WebPushD::Daemon::connectionAdded):

• webpushd/webpushtool/WebPushToolConnection.h:
• webpushd/webpushtool/WebPushToolConnection.mm:

(WebPushTool::Connection::create):
(WebPushTool::Connection::Connection):
(WebPushTool::Connection::connectToService):
(WebPushTool::Connection::connectionDropped):

• webpushd/webpushtool/WebPushToolMain.mm:

(printUsageAndTerminate):
(main):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm:

(TestWebKitAPI::TEST): Adapt to debug message changes.

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/webpushd/AppBundleRequest.mm (modified) (4 diffs)
• Source/WebKit/webpushd/PushClientConnection.h (modified) (1 diff)
• Source/WebKit/webpushd/PushClientConnection.mm (modified) (3 diffs)
• Source/WebKit/webpushd/WebPushDaemon.mm (modified) (2 diffs)
• Source/WebKit/webpushd/webpushtool/WebPushToolConnection.h (modified) (2 diffs)
• Source/WebKit/webpushd/webpushtool/WebPushToolConnection.mm (modified) (5 diffs)
• Source/WebKit/webpushd/webpushtool/WebPushToolMain.mm (modified) (5 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-12-06  Brady Eidson  <beidson@apple.com>
+
+        webpushd/webpushtool debugging additions
+        https://bugs.webkit.org/show_bug.cgi?id=233864
+
+        Reviewed by Alex Christensen.
+
+        Covered by API tests.
+        
+        This patch:
+        - Teaches webpushtool the ability to wait for a reconnect after losing its connection
+        - Starts actually broadcasting meaningful debug messages from webpushd
+
+        * webpushd/AppBundleRequest.mm:
+        (WebPushD::AppBundleRequest::start):
+        (WebPushD::AppBundlePermissionsRequest::didCheckForExistingBundle):
+        (WebPushD::AppBundlePermissionsRequest::didCreateAppBundle):
+        (WebPushD::AppBundleDeletionRequest::didDeleteExistingBundleWithError):
+
+        * webpushd/PushClientConnection.h:
+        * webpushd/PushClientConnection.mm:
+        (WebPushD::ClientConnection::setDebugModeIsEnabled):
+        (WebPushD::ClientConnection::broadcastDebugMessage):
+        (WebPushD::ClientConnection::connectionClosed):
+
+        * webpushd/WebPushDaemon.mm:
+        (WebPushD::Daemon::connectionAdded):
+
+        * webpushd/webpushtool/WebPushToolConnection.h:
+        * webpushd/webpushtool/WebPushToolConnection.mm:
+        (WebPushTool::Connection::create):
+        (WebPushTool::Connection::Connection):
+        (WebPushTool::Connection::connectToService):
+        (WebPushTool::Connection::connectionDropped):
+
+        * webpushd/webpushtool/WebPushToolMain.mm:
+        (printUsageAndTerminate):
+        (main):
+
 2021-12-06  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit/webpushd/AppBundleRequest.mm

@@ -48 +48 @@
     ASSERT(m_connection);
 
+    m_connection->broadcastDebugMessage(makeString("Starting ", transactionDescription(), " request for origin ", m_originString));
+
     m_transaction = adoptOSObject(os_transaction_create(transactionDescription()));
 
@@ -92 +94 @@
     ASSERT_UNUSED(bundle, &bundle == m_appBundle.get());
 
+    m_connection->broadcastDebugMessage(makeString("Origin ", m_originString, " app bundle request: didCheckForExistingBundle - ", exists == PushAppBundleExists::Yes ? "Exists" : "Does not exist"));
+
     if (exists == PushAppBundleExists::Yes)
         return callCompletionHandlerAndCleanup(true);
@@ -101 +105 @@
 {
     ASSERT_UNUSED(bundle, &bundle == m_appBundle.get());
+
+    m_connection->broadcastDebugMessage(makeString("Origin ", m_originString, " app bundle request: didCreateAppBundle - ", result == PushAppBundleCreationResult::Success ? "Created" : "Failed to create"));
 
     if (result == PushAppBundleCreationResult::Failure)
@@ -119 +125 @@
     ASSERT_UNUSED(bundle, &bundle == m_appBundle.get());
 
+    m_connection->broadcastDebugMessage(makeString("Origin ", m_originString, " app bundle request: didDeleteExistingBundleWithError"));
+
     if (error)
-        Daemon::singleton().broadcastDebugMessage(MessageLevel::Info, makeString("Failed to delete app bundle: ", String([error description])));
+        m_connection->broadcastDebugMessage(makeString("Failed to delete app bundle: ", String([error description])));
 
     callCompletionHandlerAndCleanup(error ? String([error description]) : "");

trunk/Source/WebKit/webpushd/PushClientConnection.h

@@ -68 +68 @@
     void connectionClosed();
 
+    void broadcastDebugMessage(const String&);
+
 private:
     ClientConnection(xpc_connection_t);

trunk/Source/WebKit/webpushd/PushClientConnection.mm

@@ -32 +32 @@
 #import "WebPushDaemonConnectionConfiguration.h"
 #import <JavaScriptCore/ConsoleTypes.h>
+#import <wtf/HexNumber.h>
 #import <wtf/Vector.h>
 #import <wtf/cocoa/Entitlements.h>
@@ -104 +105 @@
 
     m_debugModeEnabled = enabled;
+    broadcastDebugMessage(makeString("Turned Debug Mode ", m_debugModeEnabled ? "on" : "off"));
+}
 
-    auto identifier = hostAppCodeSigningIdentifier();
-    String message;
-    if (!identifier.isEmpty())
-        message = makeString("[webpushd - ", identifier, "] Turned Debug Mode ", m_debugModeEnabled ? "on" : "off");
+void ClientConnection::broadcastDebugMessage(const String& message)
+{
+    String messageIdentifier;
+    auto signingIdentifer = hostAppCodeSigningIdentifier();
+    if (signingIdentifer.isEmpty())
+        messageIdentifier = makeString ("[(0x", hex(reinterpret_cast<uint64_t>(m_xpcConnection.get()), WTF::HexConversionMode::Lowercase), ")] ");
     else
-        message = makeString("[webpushd] Turned Debug Mode ", m_debugModeEnabled ? "on" : "off");
+        messageIdentifier = makeString ("[", signingIdentifer, " (0x", hex(reinterpret_cast<uint64_t>(m_xpcConnection.get()), WTF::HexConversionMode::Lowercase), ")] ");
 
-    Daemon::singleton().broadcastDebugMessage(MessageLevel::Info, message);
+    Daemon::singleton().broadcastDebugMessage(JSC::MessageLevel::Info, makeString(messageIdentifier, message));
 }
 
@@ -146 +151 @@
 void ClientConnection::connectionClosed()
 {
+    broadcastDebugMessage("Connection closed");
+
     RELEASE_ASSERT(m_xpcConnection);
     m_xpcConnection = nullptr;

trunk/Source/WebKit/webpushd/WebPushDaemon.mm

@@ -35 +35 @@
 
 #import <wtf/CompletionHandler.h>
+#import <wtf/HexNumber.h>
 #import <wtf/NeverDestroyed.h>
 #import <wtf/Span.h>
@@ -181 +182 @@
 void Daemon::connectionAdded(xpc_connection_t connection)
 {
+    broadcastDebugMessage((JSC::MessageLevel)0, makeString("New connection: 0x", hex(reinterpret_cast<uint64_t>(connection), WTF::HexConversionMode::Lowercase)));
+
     RELEASE_ASSERT(!m_connectionMap.contains(connection));
     m_connectionMap.set(connection, ClientConnection::create(connection));

trunk/Source/WebKit/webpushd/webpushtool/WebPushToolConnection.h

@@ -37 +37 @@
 };
 
+enum class PreferTestService : bool {
+    Yes,
+    No,
+};
+
+enum class Reconnect : bool {
+    Yes,
+    No,
+};
+
 class Connection : public CanMakeWeakPtr<Connection> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    static std::unique_ptr<Connection> create(Action, bool preferTestService);
-    Connection(Action, bool preferTestService);
+    static std::unique_ptr<Connection> create(Action, PreferTestService, Reconnect);
+    Connection(Action, PreferTestService, Reconnect);
 
     void connectToService();
@@ -55 +65 @@
     
     Action m_action;
+    bool m_reconnect { false };
     RetainPtr<xpc_connection_t> m_connection;
     const char* m_serviceName;

trunk/Source/WebKit/webpushd/webpushtool/WebPushToolConnection.mm

@@ -30 +30 @@
 #import <mach/task.h>
 #import <pal/spi/cocoa/ServersSPI.h>
+#import <wtf/MainThread.h>
 #import <wtf/RetainPtr.h>
 
 namespace WebPushTool {
 
-std::unique_ptr<Connection> Connection::create(Action action, bool preferTestService)
+std::unique_ptr<Connection> Connection::create(Action action, PreferTestService preferTestService, Reconnect reconnect)
 {
-    return makeUnique<Connection>(action, preferTestService);
+    return makeUnique<Connection>(action, preferTestService, reconnect);
 }
 
@@ -53 +54 @@
 }
 
-Connection::Connection(Action action, bool preferTestService)
+Connection::Connection(Action action, PreferTestService preferTestService, Reconnect reconnect)
     : m_action(action)
+    , m_reconnect(reconnect == Reconnect::Yes)
 {
-    if (preferTestService)
+    if (preferTestService == PreferTestService::Yes)
         m_serviceName = "org.webkit.webpushtestdaemon.service";
     else
         m_serviceName = "com.apple.webkit.webpushd.service";
+}
+
+void Connection::connectToService()
+{
+    if (m_connection)
+        return;
 
     m_connection = adoptNS(xpc_connection_create_mach_service(m_serviceName, dispatch_get_main_queue(), 0));
@@ -75 +83 @@
         if (event == XPC_ERROR_CONNECTION_INTERRUPTED) {
             printf("Connection closed\n");
+            if (m_reconnect)
+                printf("===============\nReconnecting...\n");
             connectionDropped();
             return;
@@ -86 +96 @@
         RELEASE_ASSERT_NOT_REACHED();
     });
-}
-
-void Connection::connectToService()
-{
-    if (!m_connection)
-        return;
 
     auto result = maybeConnectToService(m_serviceName);
@@ -155 +159 @@
 {
     m_connection = nullptr;
+    if (m_reconnect) {
+        callOnMainRunLoop([this, weakThis = WeakPtr { this }] {
+            if (weakThis)
+                connectToService();
+        });
+        return;
+    }
+
     CFRunLoopStop(CFRunLoopGetCurrent());
 }

trunk/Source/WebKit/webpushd/webpushtool/WebPushToolMain.mm

@@ -28 +28 @@
 #import <Foundation/Foundation.h>
 #import <optional>
+#import <wtf/MainThread.h>
 
 __attribute__((__noreturn__))
@@ -39 +40 @@
     fprintf(stderr, "  --production               Connects to mach service \"com.apple.webkit.webpushd.service\"\n");
     fprintf(stderr, "  --streamDebugMessages      Stream debug messages from webpushd\n");
+    fprintf(stderr, "  --reconnect                Reconnect after connection is lost\n");
     fprintf(stderr, "\n");
 
@@ -46 +48 @@
 int main(int, const char **)
 {
-    bool preferTestService = true;
+    WTF::initializeMainThread();
+
+    auto preferTestService = WebPushTool::PreferTestService::Yes;
+    auto reconnect = WebPushTool::Reconnect::No;
     std::optional<WebPushTool::Action> action;
 
@@ -56 +61 @@
         for (NSString *argument in [arguments subarrayWithRange:NSMakeRange(1, arguments.count - 1)]) {
             if ([argument isEqualToString:@"--production"])
-                preferTestService = false;
-            if ([argument isEqualToString:@"--development"])
-                preferTestService = true;
+                preferTestService = WebPushTool::PreferTestService::No;
+            else if ([argument isEqualToString:@"--development"])
+                preferTestService = WebPushTool::PreferTestService::Yes;
             else if ([argument isEqualToString:@"--streamDebugMessages"])
                 action = WebPushTool::Action::StreamDebugMessages;
+            else if ([argument isEqualToString:@"--reconnect"])
+                reconnect = WebPushTool::Reconnect::Yes;
             else
                 printUsageAndTerminate([NSString stringWithFormat:@"Invalid option provided: %@", argument]);
@@ -69 +76 @@
         printUsageAndTerminate(@"No action provided");
 
-    auto connection = WebPushTool::Connection::create(*action, preferTestService);
+    auto connection = WebPushTool::Connection::create(*action, preferTestService, reconnect);
     connection->connectToService();
 

trunk/Tools/ChangeLog

@@ +1 @@
+2021-12-06  Brady Eidson  <beidson@apple.com>
+
+        webpushd/webpushtool debugging additions
+        https://bugs.webkit.org/show_bug.cgi?id=233864
+
+        Reviewed by Alex Christensen.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm:
+        (TestWebKitAPI::TEST): Adapt to debug message changes.
+
 2021-12-04  Jonathan Bedard  <jbedard@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm

@@ -182 +182 @@
             return;
 
-        bool stringMatches = !strcmp(debugMessage, "[webpushd - TestWebKitAPI] Turned Debug Mode on");
-        if (!stringMatches)
-            stringMatches = !strcmp(debugMessage, "[webpushd - com.apple.WebKit.TestWebKitAPI] Turned Debug Mode on");
+        NSString *nsMessage = [NSString stringWithUTF8String:debugMessage];
+
+        // Ignore possible connections/messages from webpushtool
+        if ([nsMessage hasPrefix:@"[webpushtool "])
+            return;
+
+        bool stringMatches = [nsMessage hasPrefix:@"[com.apple.WebKit.TestWebKitAPI"] || [nsMessage hasPrefix:@"[TestWebKitAPI"];
+        stringMatches = stringMatches && [nsMessage hasSuffix:@" Turned Debug Mode on"];
 
         EXPECT_TRUE(stringMatches);