Remove target interceptors in favor of session events (#2397)

* Remove target interceptors in favor of session events

* remove urlprefix from command line

* remove extra call

* event fixes

* some tweaks

* refactor some code

* Maybe firefox needs more time

Author: kblok

.github/workflows/on-push-do-docs.yml

@@ -9,7 +9,7 @@ jobs:
     - name: Run MarkdownSnippets
       run: |
         dotnet tool install --global MarkdownSnippets.Tool
-        mdsnippets ${GITHUB_WORKSPACE} --url-prefix https://github.com/hardkoded/puppeteer-sharp/blob/master
+        mdsnippets ${GITHUB_WORKSPACE}
       shell: bash
     - name: Push changes
       run: |

lib/PuppeteerSharp.Tests/test.runsettings

@@ -2,6 +2,6 @@
 <RunSettings>
   <!-- Configurations that affect the Test Framework -->
   <RunConfiguration>
-    <TestSessionTimeout>2700000</TestSessionTimeout>
+    <TestSessionTimeout>3600000</TestSessionTimeout>
   </RunConfiguration>
 </RunSettings>

lib/PuppeteerSharp/Browser.cs

@@ -392,7 +392,7 @@ private async void OnAttachedToTargetAsync(object sender, TargetChangedArgs e)
         {
             try
             {
-                if ((await e.Target.InitializedTask.ConfigureAwait(false)) == InitializationStatus.Success)
+                if (await e.Target.InitializedTask.ConfigureAwait(false) == InitializationStatus.Success)
                 {
                     var args = new TargetChangedArgs { Target = e.Target };
                     TargetCreated?.Invoke(this, args);

lib/PuppeteerSharp/BrowserData/Firefox.cs

@@ -11,7 +11,7 @@ namespace PuppeteerSharp.BrowserData
     public static class Firefox
     {
         /// <summary>
-        /// Default firefoxbuild.
+        /// Default firefox build.
         /// </summary>
         public const string DefaultBuildId = "FIREFOX_NIGHTLY";
 
@@ -24,9 +24,9 @@ internal static string ResolveDownloadUrl(Platform platform, string buildId, str
 
         internal static async Task<string> ResolveBuildIdAsync(string channel)
         {
-            if (_cachedBuildIds.ContainsKey(channel))
+            if (_cachedBuildIds.TryGetValue(channel, out var build))
             {
-                return _cachedBuildIds[channel];
+                return build;
             }
 
             var version = await JsonUtils.GetAsync<Dictionary<string, string>>("https://product-details.mozilla.org/1.0/firefox_versions.json").ConfigureAwait(false);
@@ -40,7 +40,7 @@ internal static async Task<string> ResolveBuildIdAsync(string channel)
             return version[channel];
         }
 
-        internal static string RelativeExecutablePath(Platform platform, string builId)
+        internal static string RelativeExecutablePath(Platform platform, string buildId)
             => platform switch
             {
                 Platform.MacOS or Platform.MacOSArm64 => Path.Combine(

lib/PuppeteerSharp/CDPSession.cs

@@ -34,6 +34,8 @@ internal CDPSession(Connection connection, TargetType targetType, string session
         /// <inheritdoc/>
         public event EventHandler<SessionEventArgs> SessionDetached;
 
+        internal event EventHandler<SessionEventArgs> Ready;
+
         /// <inheritdoc/>
         public TargetType TargetType { get; }
 
@@ -49,9 +51,10 @@ internal CDPSession(Connection connection, TargetType targetType, string session
         /// <inheritdoc/>
         public ILoggerFactory LoggerFactory => Connection.LoggerFactory;
 
-        /// <inheritdoc cref="Connection"/>
         internal Connection Connection { get; private set; }
 
+        internal Target Target { get; set; }
+
         /// <inheritdoc/>
         public async Task<T> SendAsync<T>(string method, object args = null)
         {
@@ -117,6 +120,8 @@ public Task DetachAsync()
 
         internal bool HasPendingCallbacks() => !_callbacks.IsEmpty;
 
+        internal void OnSessionReady(CDPSession session) => Ready?.Invoke(this, new SessionEventArgs(session));
+
         internal void OnMessage(ConnectionResponse obj)
         {
             var id = obj.Id;
@@ -159,11 +164,11 @@ internal void Close(string closeReason)
         }
 
         internal void OnSessionAttached(CDPSession session)
-            => SessionAttached?.Invoke(this, new SessionEventArgs { Session = session });
+            => SessionAttached?.Invoke(this, new SessionEventArgs(session));
 
         internal void OnSessionDetached(CDPSession session)
-            => SessionDetached?.Invoke(this, new SessionEventArgs { Session = session });
+            => SessionDetached?.Invoke(this, new SessionEventArgs(session));
 
-        internal ICollection<MessageTask> GetPendingMessages() => _callbacks.Values;
+        internal IEnumerable<MessageTask> GetPendingMessages() => _callbacks.Values;
     }
 }

lib/PuppeteerSharp/ChromeTargetManager.cs

@@ -19,8 +19,7 @@ internal class ChromeTargetManager : ITargetManager
         private readonly AsyncDictionaryHelper<string, Target> _attachedTargetsByTargetId = new("Target {0} not found");
         private readonly ConcurrentDictionary<string, Target> _attachedTargetsBySessionId = new();
         private readonly ConcurrentDictionary<string, TargetInfo> _discoveredTargetsByTargetId = new();
-        private readonly ConcurrentDictionary<ICDPConnection, List<TargetInterceptor>> _targetInterceptors = new();
-        private readonly List<string> _targetsIdsForInit = new();
+        private readonly List<string> _targetsIdsForInit = [];
         private readonly TaskCompletionSource<bool> _initializeCompletionSource = new();
 
         // Needed for .NET only to prevent race conditions between StoreExistingTargetsForInit and OnAttachedToTarget
@@ -100,18 +99,6 @@ public async Task InitializeAsync()
             await _initializeCompletionSource.Task.ConfigureAwait(false);
         }
 
-        public void AddTargetInterceptor(CDPSession session, TargetInterceptor interceptor)
-        {
-            var interceptors = _targetInterceptors.GetOrAdd(session, static _ => new());
-            interceptors.Add(interceptor);
-        }
-
-        public void RemoveTargetInterceptor(CDPSession session, TargetInterceptor interceptor)
-        {
-            _targetInterceptors.TryGetValue(session, out var interceptors);
-            interceptors?.Remove(interceptor);
-        }
-
         private void StoreExistingTargetsForInit()
         {
             foreach (var kv in _discoveredTargetsByTargetId)
@@ -179,7 +166,6 @@ private void OnMessageReceived(object sender, MessageEventArgs e)
         private void Connection_SessionDetached(object sender, SessionEventArgs e)
         {
             e.Session.MessageReceived -= OnMessageReceived;
-            _targetInterceptors.TryRemove(e.Session, out var _);
         }
 
         private void OnTargetCreated(TargetCreatedResponse e)
@@ -247,35 +233,17 @@ private void OnTargetInfoChanged(TargetCreatedResponse e)
 
         private async Task OnAttachedToTargetAsync(object sender, TargetAttachedToTargetResponse e)
         {
-            var parent = sender as ICDPConnection;
+            var parentSession = sender as ICDPConnection;
+
             var targetInfo = e.TargetInfo;
             var session = _connection.GetSession(e.SessionId) ?? throw new PuppeteerException($"Session {e.SessionId} was not created.");
 
-            async Task SilentDetach()
-            {
-                try
-                {
-                    await session.SendAsync("Runtime.runIfWaitingForDebugger").ConfigureAwait(false);
-                    await parent.SendAsync(
-                        "Target.detachFromTarget",
-                        new TargetDetachFromTargetRequest
-                        {
-                            SessionId = session.Id,
-                        }).ConfigureAwait(false);
-                }
-                catch (Exception ex)
-                {
-                    _logger.LogError(ex, "silentDetach failed.");
-                }
-            }
-
             if (!_connection.IsAutoAttached(targetInfo.TargetId))
             {
                 return;
             }
 
-            if (targetInfo.Type == TargetType.ServiceWorker &&
-                _connection.IsAutoAttached(targetInfo.TargetId))
+            if (targetInfo.Type == TargetType.ServiceWorker)
             {
                 await EnsureTargetsIdsForInitAsync().ConfigureAwait(false);
                 FinishInitializationIfReady(targetInfo.TargetId);
@@ -292,8 +260,8 @@ await parent.SendAsync(
                 return;
             }
 
-            var existingTarget = _attachedTargetsByTargetId.TryGetValue(targetInfo.TargetId, out var target);
-            if (!existingTarget)
+            var isExistingTarget = _attachedTargetsByTargetId.TryGetValue(targetInfo.TargetId, out var target);
+            if (!isExistingTarget)
             {
                 target = _targetFactoryFunc(targetInfo, session);
             }
@@ -309,8 +277,9 @@ await parent.SendAsync(
 
             session.MessageReceived += OnMessageReceived;
 
-            if (existingTarget)
+            if (isExistingTarget)
             {
+                session.Target = target;
                 _attachedTargetsBySessionId.TryAdd(session.Id, target);
             }
             else
@@ -321,24 +290,12 @@ await parent.SendAsync(
                 _attachedTargetsBySessionId.TryAdd(session.Id, target);
             }
 
-            if (_targetInterceptors.TryGetValue(parent, out var interceptors))
-            {
-                foreach (var interceptor in interceptors)
-                {
-                    Target parentTarget = null;
-                    if (parent is CDPSession parentSession && !_attachedTargetsBySessionId.TryGetValue(parentSession.Id, out parentTarget))
-                    {
-                        throw new PuppeteerException("Parent session not found in attached targets");
-                    }
-
-                    interceptor(target, parentTarget);
-                }
-            }
+            (parentSession as CDPSession)?.OnSessionReady(session);
 
             await EnsureTargetsIdsForInitAsync().ConfigureAwait(false);
             _targetsIdsForInit.Remove(target.TargetId);
 
-            if (!existingTarget)
+            if (!isExistingTarget)
             {
                 TargetAvailable?.Invoke(this, new TargetChangedArgs { Target = target });
             }
@@ -360,6 +317,26 @@ await Task.WhenAll(
             {
                 _logger.LogError(ex, "Failed to call setAutoAttach and runIfWaitingForDebugger");
             }
+
+            return;
+
+            async Task SilentDetach()
+            {
+                try
+                {
+                    await session.SendAsync("Runtime.runIfWaitingForDebugger").ConfigureAwait(false);
+                    await parentSession!.SendAsync(
+                        "Target.detachFromTarget",
+                        new TargetDetachFromTargetRequest
+                        {
+                            SessionId = session.Id,
+                        }).ConfigureAwait(false);
+                }
+                catch (Exception ex)
+                {
+                    _logger.LogError(ex, "silentDetach failed.");
+                }
+            }
         }
 
         private async Task OnAttachedToTargetHandlingExceptionsAsync(object sender, string messageId, TargetAttachedToTargetResponse e)

lib/PuppeteerSharp/Connection.cs

@@ -306,7 +306,7 @@ private void ProcessIncomingMessage(ConnectionResponse obj)
                 var session = new CDPSession(this, param.TargetInfo.Type, sessionId);
                 _sessions.AddItem(sessionId, session);
 
-                SessionAttached?.Invoke(this, new SessionEventArgs { Session = session });
+                SessionAttached?.Invoke(this, new SessionEventArgs(session));
 
                 if (obj.SessionId != null && _sessions.TryGetValue(obj.SessionId, out var parentSession))
                 {
@@ -319,7 +319,7 @@ private void ProcessIncomingMessage(ConnectionResponse obj)
                 if (_sessions.TryRemove(sessionId, out var session) && !session.IsClosed)
                 {
                     session.Close("Target.detachedFromTarget");
-                    SessionDetached?.Invoke(this, new SessionEventArgs() { Session = session });
+                    SessionDetached?.Invoke(this, new SessionEventArgs(session));
 
                     if (_sessions.TryGetValue(sessionId, out var parentSession))
                     {

lib/PuppeteerSharp/FirefoxTargetManager.cs

@@ -16,13 +16,11 @@ internal class FirefoxTargetManager : ITargetManager
         private readonly Func<TargetInfo, CDPSession, Target> _targetFactoryFunc;
         private readonly Func<Target, bool> _targetFilterFunc;
         private readonly ILogger<FirefoxTargetManager> _logger;
-        private readonly ConcurrentDictionary<ICDPConnection, List<TargetInterceptor>> _targetInterceptors = new();
         private readonly AsyncDictionaryHelper<string, Target> _availableTargetsByTargetId = new("Target {0} not found");
         private readonly ConcurrentDictionary<string, Target> _availableTargetsBySessionId = new();
         private readonly ConcurrentDictionary<string, TargetInfo> _discoveredTargetsByTargetId = new();
         private readonly TaskCompletionSource<bool> _initializeCompletionSource = new();
-        private readonly List<string> _ignoredTargets = new();
-        private List<string> _targetsIdsForInit = new();
+        private List<string> _targetsIdsForInit = [];
 
         public FirefoxTargetManager(
             Connection connection,
@@ -45,19 +43,6 @@ public FirefoxTargetManager(
 
         public event EventHandler<TargetChangedArgs> TargetDiscovered;
 
-        public void AddTargetInterceptor(CDPSession session, TargetInterceptor interceptor)
-        {
-            var interceptors = _targetInterceptors.GetOrAdd(session, static _ => new());
-            interceptors.Add(interceptor);
-        }
-
-        public void RemoveTargetInterceptor(CDPSession session, TargetInterceptor interceptor)
-        {
-            _targetInterceptors.TryGetValue(session, out var interceptors);
-
-            interceptors?.Remove(interceptor);
-        }
-
         public async Task InitializeAsync()
         {
             await _connection.SendAsync("Target.setDiscoverTargets", new TargetSetDiscoverTargetsRequest
@@ -69,7 +54,7 @@ public async Task InitializeAsync()
                 },
             }).ConfigureAwait(false);
 
-            _targetsIdsForInit = new List<string>(_discoveredTargetsByTargetId.Keys);
+            _targetsIdsForInit = [.._discoveredTargetsByTargetId.Keys];
             await _initializeCompletionSource.Task.ConfigureAwait(false);
         }
 
@@ -102,10 +87,7 @@ private void OnMessageReceived(object sender, MessageEventArgs e)
         }
 
         private void Connection_SessionDetached(object sender, SessionEventArgs e)
-        {
-            e.Session.MessageReceived -= OnMessageReceived;
-            _targetInterceptors.TryRemove(e.Session, out var _);
-        }
+            => e.Session.MessageReceived -= OnMessageReceived;
 
         private void OnTargetCreated(TargetCreatedResponse e)
         {
@@ -125,7 +107,6 @@ private void OnTargetCreated(TargetCreatedResponse e)
             var target = _targetFactoryFunc(e.TargetInfo, null);
             if (_targetFilterFunc != null && !_targetFilterFunc(target))
             {
-                _ignoredTargets.Add(e.TargetInfo.TargetId);
                 FinishInitializationIfReady(e.TargetInfo.TargetId);
                 return;
             }
@@ -155,27 +136,15 @@ private void OnTargetDestroyed(TargetDestroyedResponse e)
 
         private void OnAttachedToTarget(object sender, TargetAttachedToTargetResponse e)
         {
-            var parent = sender as ICDPConnection;
+            var parentSession = sender as CDPSession;
             var targetInfo = e.TargetInfo;
             var session = _connection.GetSession(e.SessionId) ?? throw new PuppeteerException($"Session {e.SessionId} was not created.");
-            var existingTarget = _availableTargetsByTargetId.TryGetValue(targetInfo.TargetId, out var target);
+            _availableTargetsByTargetId.TryGetValue(targetInfo.TargetId, out var target);
             session.MessageReceived += OnMessageReceived;
-
+            session.Target = target;
             _availableTargetsBySessionId.TryAdd(session.Id, target);
 
-            if (_targetInterceptors.TryGetValue(parent, out var interceptors))
-            {
-                foreach (var interceptor in interceptors)
-                {
-                    Target parentTarget = null;
-                    if (parent is CDPSession parentSession && !_availableTargetsBySessionId.TryGetValue(parentSession.Id, out parentTarget))
-                    {
-                        throw new PuppeteerException("Parent session not found in attached targets");
-                    }
-
-                    interceptor(target, parentTarget);
-                }
-            }
+            parentSession?.OnSessionReady(session);
         }
 
         private void FinishInitializationIfReady(string targetId = null)

lib/PuppeteerSharp/ICDPSession.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
-using Newtonsoft.Json.Linq;
 
 namespace PuppeteerSharp
 {