Cherry-pick 272970@main (21c1140). https://bugs.webkit.org/show_bug.c…

…gi?id=267132

    [GTK][WPE][Debug] imported/w3c/web-platform-tests/notifications/instance.https.html is a constant crash
    https://bugs.webkit.org/show_bug.cgi?id=267132

    Reviewed by Carlos Garcia Campos.

    Currently, when `NotificationResourcesLoader` starts downloading
    notification resources, we create a "ResourceLoader" per resource
    (we only support the icon resource for now) and add it to the set of
    running loaders `m_loaders`.

    When a `ResourceLoader` finishes, we check if it was the last one
    running. If yes, we run the `NotificationResourcesLoader`'s completion
    handler.

    But in some circumstances (e.g. when the icon URL is bogus)
    a `ResouceLoader` can execute its completion handler synchronously
    during construction.

    In this case, the `NotificationResourcesLoader`'s completion handler
    will be called from `didFinishLoadingResource()` because
    `m_loaders.isEmpty()` since we haven't added the loader to the set yet.

    To prevent it, this patch checks that `ResouceLoader` hasn't finished
    yet before adding it to the set of running loaders.

    * Source/WebCore/Modules/notifications/NotificationResourcesLoader.cpp:
    (WebCore::NotificationResourcesLoader::start):
    (WebCore::NotificationResourcesLoader::stop):
    (WebCore::NotificationResourcesLoader::didFinishLoadingResource):
    (WebCore::NotificationResourcesLoader::ResourceLoader::didFinishLoading):
    (WebCore::NotificationResourcesLoader::ResourceLoader::didFail):
    * Source/WebCore/Modules/notifications/NotificationResourcesLoader.h:

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

Source/WebCore/Modules/notifications/NotificationResourcesLoader.cpp

@@ -73,27 +73,37 @@ void NotificationResourcesLoader::start(CompletionHandler<void(RefPtr<Notificati
     if (resourceIsSupportedInPlatform(Resource::Icon)) {
         const URL& iconURL = m_notification.icon();
         if (!iconURL.isEmpty()) {
-            if (!m_resources)
-                m_resources = NotificationResources::create();
-            m_loaders.add(makeUnique<ResourceLoader>(*m_notification.scriptExecutionContext(), iconURL, [this](ResourceLoader* loader, RefPtr<BitmapImage>&& image) {
-                if (!m_resources)
+            auto loader = makeUnique<ResourceLoader>(*m_notification.scriptExecutionContext(), iconURL, [this](ResourceLoader* loader, RefPtr<BitmapImage>&& image) {
+                if (m_stopped)
                     return;
 
-                m_resources->setIcon(WTFMove(image));
+                if (image) {
+                    if (!m_resources)
+                        m_resources = NotificationResources::create();
+                    m_resources->setIcon(WTFMove(image));
+                }
+
                 didFinishLoadingResource(loader);
-            }));
+            });
+
+            if (!loader->finished())
+                m_loaders.add(WTFMove(loader));
         }
     }
 
     // FIXME: Implement other resources.
 
     if (m_loaders.isEmpty())
-        m_completionHandler(nullptr);
+        m_completionHandler(WTFMove(m_resources));
 }
 
 void NotificationResourcesLoader::stop()
 {
-    m_resources = nullptr;
+    if (m_stopped)
+        return;
+
+    m_stopped = true;
+
     auto completionHandler = std::exchange(m_completionHandler, nullptr);
 
     while (!m_loaders.isEmpty()) {
@@ -107,9 +117,11 @@ void NotificationResourcesLoader::stop()
 
 void NotificationResourcesLoader::didFinishLoadingResource(ResourceLoader* loader)
 {
-    m_loaders.remove(loader);
-    if (m_loaders.isEmpty() && m_completionHandler)
-        m_completionHandler(WTFMove(m_resources));
+    if (m_loaders.contains(loader)) {
+        m_loaders.remove(loader);
+        if (m_loaders.isEmpty() && m_completionHandler)
+            m_completionHandler(WTFMove(m_resources));
+    }
 }
 
 NotificationResourcesLoader::ResourceLoader::ResourceLoader(ScriptExecutionContext& context, const URL& url, CompletionHandler<void(ResourceLoader*, RefPtr<BitmapImage>&&)>&& completionHandler)
@@ -153,6 +165,8 @@ void NotificationResourcesLoader::ResourceLoader::didReceiveData(const SharedBuf
 
 void NotificationResourcesLoader::ResourceLoader::didFinishLoading(ResourceLoaderIdentifier, const NetworkLoadMetrics&)
 {
+    m_finished = true;
+
     if (m_image)
         m_image->setData(m_buffer.take(), true);
 
@@ -162,6 +176,8 @@ void NotificationResourcesLoader::ResourceLoader::didFinishLoading(ResourceLoade
 
 void NotificationResourcesLoader::ResourceLoader::didFail(const ResourceError&)
 {
+    m_finished = true;
+
     if (m_completionHandler)
         m_completionHandler(this, nullptr);
 }

Source/WebCore/Modules/notifications/NotificationResourcesLoader.h

@@ -61,13 +61,16 @@ class NotificationResourcesLoader {
 
         void cancel();
 
+        bool finished() const { return m_finished; }
+
     private:
         // ThreadableLoaderClient API.
         void didReceiveResponse(ResourceLoaderIdentifier, const ResourceResponse&) final;
         void didReceiveData(const SharedBuffer&) final;
         void didFinishLoading(ResourceLoaderIdentifier, const NetworkLoadMetrics&) final;
         void didFail(const ResourceError&) final;
 
+        bool m_finished { false };
         SharedBufferBuilder m_buffer;
         RefPtr<BitmapImage> m_image;
         RefPtr<ThreadableLoader> m_loader;
@@ -77,6 +80,7 @@ class NotificationResourcesLoader {
     void didFinishLoadingResource(ResourceLoader*);
 
     Notification& m_notification;
+    bool m_stopped { false };
     CompletionHandler<void(RefPtr<NotificationResources>&&)> m_completionHandler;
     HashSet<std::unique_ptr<ResourceLoader>> m_loaders;
     RefPtr<NotificationResources> m_resources;