Cherry-pick 53b1e6b. rdar://123423266

    1Password fails to load popup after focusing a form field on a page.
    https://webkit.org/b/270233
    rdar://123423266

    Reviewed by Jeff Miller and Brian Weinstein.

    Once the input field is focused, 1Password creates an iframe with an extension document in the
    tab. This causes two processes to have listeners for the runtime.onMessage event. That then
    causes a race when sending messages, and the tab frame wins since the background replies async
    after doing some work.

    This was happening because the reply aggregator in the web process was sending a default null
    reply, since the completion handler is always called, which was indistinguishable from a real
    reply. Now we always send either null for default or at minimum an empty string for the JSON
    reply, and the UI process skips the replies that are null strings. The aggregator on the UI
    process side will then get the real reply or default to null later once it goes out of scope.

    * Source/WebKit/UIProcess/Extensions/Cocoa/API/WebExtensionContextAPIRuntimeCocoa.mm:
    (WebKit::WebExtensionContext::runtimeSendMessage): Don't call callbackAggregator for null string.
    (WebKit::WebExtensionContext::runtimeWebPageSendMessage): Ditto.
    * Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIRuntimeCocoa.mm:
    (WebKit::WebExtensionContextProxy::internalDispatchRuntimeMessageEvent): Ensure a real reply is
    never null, so the completionHandler can make the distinction. Send default replies as null.
    * Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIRuntime.mm:
    (TEST(WKWebExtensionAPIRuntime, SendMessageWithTabFrameAndAsyncReply)): Added.
    (TEST(WKWebExtensionAPIRuntime, SendMessageFromWebPageWithTabFrameAndAsyncReply)): Added.

    Canonical link: https://commits.webkit.org/275461@main

Source/WebKit/UIProcess/Extensions/Cocoa/API/WebExtensionContextAPIRuntimeCocoa.mm

@@ -145,6 +145,11 @@
 
     for (auto& process : mainWorldProcesses) {
         process->sendWithAsyncReply(Messages::WebExtensionContextProxy::DispatchRuntimeMessageEvent(targetContentWorldType, messageJSON, std::nullopt, completeSenderParameters), [callbackAggregator](String&& replyJSON) {
+            // A null reply means no listeners replied. Don't call the callbackAggregator
+            // to give other listeners in a different process a chance to reply.
+            if (replyJSON.isNull())
+                return;
+
             callbackAggregator.get()(WTFMove(replyJSON));
         }, identifier());
     }
@@ -294,6 +299,11 @@
 
     for (auto& process : mainWorldProcesses) {
         process->sendWithAsyncReply(Messages::WebExtensionContextProxy::DispatchRuntimeMessageEvent(WebExtensionContentWorldType::Main, messageJSON, std::nullopt, completeSenderParameters), [callbackAggregator](String&& replyJSON) {
+            // A null reply means no listeners replied. Don't call the callbackAggregator
+            // to give other listeners in a different process a chance to reply.
+            if (replyJSON.isNull())
+                return;
+
             callbackAggregator.get()(WTFMove(replyJSON));
         }, identifier());
     }

Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIRuntimeCocoa.mm

@@ -59,7 +59,8 @@
 
 namespace WebKit {
 
-using ReplyCallbackAggregator = EagerCallbackAggregator<void(id)>;
+enum class IsDefaultReply : bool { No, Yes };
+using ReplyCallbackAggregator = EagerCallbackAggregator<void(id, IsDefaultReply)>;
 
 }
 
@@ -552,18 +553,28 @@ - (instancetype)initWithAggregator:(WebKit::ReplyCallbackAggregator&)aggregator
     auto *senderInfo = toWebAPI(senderParameters);
     auto sourceContentWorldType = senderParameters.contentWorldType;
 
-    auto callbackAggregator = ReplyCallbackAggregator::create([completionHandler = WTFMove(completionHandler)](id replyMessage) mutable {
+    auto callbackAggregator = ReplyCallbackAggregator::create([completionHandler = WTFMove(completionHandler)](id replyMessage, IsDefaultReply defaultReply) mutable {
+        if (defaultReply == IsDefaultReply::Yes) {
+            // A null reply to the completionHandler means no listeners replied.
+            completionHandler({ });
+            return;
+        }
+
         NSString *replyMessageJSON;
         if (JSValue *value = dynamic_objc_cast<JSValue>(replyMessage))
             replyMessageJSON = value._toJSONString;
         else
             replyMessageJSON = encodeJSONString(replyMessage, JSONOptions::FragmentsAllowed);
 
         if (replyMessageJSON.length > webExtensionMaxMessageLength)
-            replyMessageJSON = nil;
+            replyMessageJSON = @"";
+
+        // Ensure a real reply is never null, so the completionHandler can make the distinction.
+        if (!replyMessageJSON)
+            replyMessageJSON = @"";
 
         completionHandler(replyMessageJSON);
-    }, nil);
+    }, nil, IsDefaultReply::Yes);
 
     // This ObjC wrapper is need for the inner reply block, which is required to be a compiled block.
     auto *callbackAggregatorWrapper = [[_WKReplyCallbackAggregator alloc] initWithAggregator:callbackAggregator];
@@ -592,7 +603,7 @@ - (instancetype)initWithAggregator:(WebKit::ReplyCallbackAggregator&)aggregator
             // with a signature to translate the JS function arguments. Having the block capture
             // callbackAggregatorWrapper ensures that callbackAggregator remains in scope.
             id returnValue = listener->call(message, senderInfo, ^(id replyMessage) {
-                callbackAggregatorWrapper.get().aggregator(replyMessage);
+                callbackAggregatorWrapper.get().aggregator(replyMessage, IsDefaultReply::No);
             });
 
             if (dynamic_objc_cast<NSNumber>(returnValue).boolValue) {
@@ -610,13 +621,13 @@ - (instancetype)initWithAggregator:(WebKit::ReplyCallbackAggregator&)aggregator
                 if (error)
                     return;
 
-                callbackAggregatorWrapper.get().aggregator(replyMessage);
+                callbackAggregatorWrapper.get().aggregator(replyMessage, IsDefaultReply::No);
             }];
         }
     }, toDOMWrapperWorld(contentWorldType));
 
     if (!anyListenerHandledMessage)
-        callbackAggregator.get()(nil);
+        callbackAggregator.get()(nil, IsDefaultReply::Yes);
 }
 
 void WebExtensionContextProxy::dispatchRuntimeMessageEvent(WebExtensionContentWorldType contentWorldType, const String& messageJSON, std::optional<WebExtensionFrameIdentifier> frameIdentifier, const WebExtensionMessageSenderParameters& senderParameters, CompletionHandler<void(String&& replyJSON)>&& completionHandler)

Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIRuntime.mm

@@ -683,6 +683,91 @@
     [manager run];
 }
 
+TEST(WKWebExtensionAPIRuntime, SendMessageWithTabFrameAndAsyncReply)
+{
+    TestWebKitAPI::HTTPServer server({
+        { "/"_s, { { { "Content-Type"_s, "text/html"_s } }, ""_s } },
+    }, TestWebKitAPI::HTTPServer::Protocol::Http);
+
+    auto *extensionManifest = @{
+        @"manifest_version": @3,
+
+        @"name": @"Runtime Test",
+        @"description": @"Runtime Test",
+        @"version": @"1",
+
+        @"background": @{
+            @"scripts": @[ @"background.js" ],
+            @"type": @"module",
+            @"persistent": @NO,
+        },
+
+        @"content_scripts": @[ @{
+            @"js": @[ @"content.js" ],
+            @"matches": @[ @"*://localhost/*" ],
+        } ],
+
+        @"web_accessible_resources": @[ @{
+            @"resources": @[ @"*.html" ],
+            @"matches": @[ @"*://localhost/*" ]
+        } ],
+    };
+
+    auto *backgroundScript = Util::constructScript(@[
+        @"browser.runtime.onMessage.addListener((message, sender, sendResponse) => {",
+        @"  if (message?.content === 'Hello from iframe')",
+        @"    setTimeout(() => sendResponse({ content: 'Async reply from background' }), 500)",
+
+        @"  return true",
+        @"})",
+
+        @"browser.test.yield('Load Tab')",
+    ]);
+
+    auto *iframeScript = Util::constructScript(@[
+        @"browser.runtime.onMessage.addListener((message, sender, sendResponse) => {",
+        @"  // This listener should not be called since it is in the same frame as the sender,",
+        @"  // but having it is needed to verify the background script is the responder.",
+
+        @"  browser.test.notifyFail('Frame listener should not be called')",
+        @"})",
+
+        @"const response = await browser.runtime.sendMessage({ content: 'Hello from iframe' })",
+        @"browser.test.assertEq(response?.content, 'Async reply from background', 'Should receive the correct async reply from background script')",
+
+        @"browser.test.notifyPass()",
+    ]);
+
+    auto *contentScript = Util::constructScript(@[
+        @"(function() {",
+        @"  const iframe = document.createElement('iframe')",
+        @"  iframe.src = browser.runtime.getURL('extension-frame.html')",
+        @"  document.body.appendChild(iframe)",
+        @"})()",
+    ]);
+
+    auto *resources = @{
+        @"background.js": backgroundScript,
+        @"content.js": contentScript,
+        @"extension-frame.js": iframeScript,
+        @"extension-frame.html": @"<script type='module' src='extension-frame.js'></script>",
+    };
+
+    auto extension = adoptNS([[_WKWebExtension alloc] _initWithManifestDictionary:extensionManifest resources:resources]);
+    auto manager = adoptNS([[TestWebExtensionManager alloc] initForExtension:extension.get()]);
+
+    auto *urlRequest = server.requestWithLocalhost();
+    [manager.get().context setPermissionStatus:_WKWebExtensionContextPermissionStatusGrantedExplicitly forURL:urlRequest.URL];
+
+    [manager loadAndRun];
+
+    EXPECT_NS_EQUAL(manager.get().yieldMessage, @"Load Tab");
+
+    [manager.get().defaultTab.mainWebView loadRequest:urlRequest];
+
+    [manager run];
+}
+
 TEST(WKWebExtensionAPIRuntime, ConnectFromContentScript)
 {
     TestWebKitAPI::HTTPServer server({
@@ -1280,6 +1365,108 @@
     [manager run];
 }
 
+TEST(WKWebExtensionAPIRuntime, SendMessageFromWebPageWithTabFrameAndAsyncReply)
+{
+    auto *extensionManifest = @{
+        @"manifest_version": @3,
+
+        @"name": @"Runtime Test",
+        @"description": @"Runtime Test",
+        @"version": @"1",
+
+        @"background": @{
+            @"scripts": @[ @"background.js" ],
+            @"type": @"module",
+            @"persistent": @NO,
+        },
+
+        @"content_scripts": @[ @{
+            @"js": @[ @"content.js" ],
+            @"matches": @[ @"*://localhost/*" ],
+        } ],
+
+        @"web_accessible_resources": @[ @{
+            @"resources": @[ @"*.html" ],
+            @"matches": @[ @"*://localhost/*" ]
+        } ],
+
+        @"externally_connectable": @{
+            @"matches": @[ @"*://localhost/*" ]
+        },
+    };
+
+    auto *backgroundScript = Util::constructScript(@[
+        @"browser.runtime.onMessageExternal.addListener((message, sender, sendResponse) => {",
+        @"  browser.test.assertEq(message?.content, 'Hello from webpage', 'Should receive the correct message from the web page')",
+
+        @"  setTimeout(() => sendResponse({ content: 'Async reply from background' }), 500)",
+
+        @"  return true",
+        @"})",
+
+        @"browser.test.yield('Load Tabs')"
+    ]);
+
+    auto *iframeScript = Util::constructScript(@[
+        @"browser.runtime.onMessageExternal.addListener((message, sender, sendResponse) => {",
+        @"  browser.test.assertEq(message?.content, 'Hello from webpage', 'Should receive the correct message from the web page')",
+        @"})",
+    ]);
+
+    auto *contentScript = Util::constructScript(@[
+        @"(function() {",
+        @"  const iframe = document.createElement('iframe')",
+        @"  iframe.src = browser.runtime.getURL('extension-frame.html')",
+        @"  document.body.appendChild(iframe)",
+        @"})()",
+    ]);
+
+    auto *webpageScript = Util::constructScript(@[
+        @"<script>",
+        @"setTimeout(() => {",
+        @"  browser.runtime.sendMessage('org.webkit.test.extension (SendMessageTest)', { content: 'Hello from webpage' }, (response) => {",
+        @"    browser.test.assertEq(response?.content, 'Async reply from background', 'Should receive the correct reply from the extension frame')",
+
+        @"    browser.test.notifyPass()",
+        @"  })",
+        @"}, 1000)",
+        @"</script>"
+    ]);
+
+    TestWebKitAPI::HTTPServer server({
+        { "/"_s, { { { "Content-Type"_s, "text/html"_s } }, webpageScript } },
+        { "/second-tab.html"_s, { { { "Content-Type"_s, "text/html"_s } }, ""_s } },
+    }, TestWebKitAPI::HTTPServer::Protocol::Http);
+
+    auto *urlRequest = server.requestWithLocalhost();
+
+    auto *resources = @{
+        @"background.js": backgroundScript,
+        @"content.js": contentScript,
+        @"extension-frame.js": iframeScript,
+        @"extension-frame.html": @"<script type='module' src='extension-frame.js'></script>",
+    };
+
+    auto extension = adoptNS([[_WKWebExtension alloc] _initWithManifestDictionary:extensionManifest resources:resources]);
+    auto manager = adoptNS([[TestWebExtensionManager alloc] initForExtension:extension.get()]);
+
+    [manager.get().context setPermissionStatus:_WKWebExtensionContextPermissionStatusGrantedExplicitly forURL:urlRequest.URL];
+
+    // Set an uniqueIdentifier so it is a known value and not the default random one.
+    manager.get().context.uniqueIdentifier = @"org.webkit.test.extension (SendMessageTest)";
+
+    [manager loadAndRun];
+
+    EXPECT_NS_EQUAL(manager.get().yieldMessage, @"Load Tabs");
+
+    [manager.get().defaultTab.mainWebView loadRequest:urlRequest];
+
+    auto *secondTab = [manager.get().defaultWindow openNewTab];
+    [secondTab.mainWebView loadRequest:server.requestWithLocalhost("/second-tab.html"_s)];
+
+    [manager run];
+}
+
 } // namespace TestWebKitAPI
 
 #endif // ENABLE(WK_WEB_EXTENSIONS)