fix(context): reuse the session of an attached target in attach_target

CHANGELOG.md

@@ -40,6 +40,7 @@
   `429 Too Many Requests` for a full remote browser service, and exposes it as `#status`
 
 ### Fixed
+- `attach_target` reuses a target's session and returns the target, instead of opening one more that reruns extensions
 - Service workers registered by a page stayed attached and alive for the browser's lifetime, now Chrome can stop them
 - An out-of-process iframe kept its parent page's session when `Target.targetCreated` came before
   `Target.attachedToTarget`, so commands sent through it reached the page [#663]

lib/ferrum/context.rb

@@ -164,18 +164,26 @@ def delete_target(target_id)
 
     # Manually attaches to a target, e.g. a service worker discovered via
     # {#service_workers}. Once attached, `target.worker`/`target.page`
-    # returns a connected {Worker}/{Page} for it.
+    # returns a connected {Worker}/{Page} for it. A target that already has
+    # a session keeps it.
     #
     # Note: attaching to a service worker's session prevents Chrome from
     # ever terminating it while the connection is open.
+    #
+    # @param [String] target_id
+    #
+    # @return [Target]
+    #
+    # @raise [NoSuchTargetError] if the context has no such target.
     def attach_target(target_id)
       target = @targets[target_id]
       raise NoSuchTargetError unless target
+      return target if target.session_id
 
       @contexts.manually_attached(target_id)
       session = @client.command("Target.attachToTarget", targetId: target_id, flatten: true)
       target.session_id = session["sessionId"]
-      true
+      target
     end
 
     # Returns the first target for which the block returns truthy.

lib/ferrum/contexts.rb

@@ -168,6 +168,10 @@ def subscribe_attached_target(client)
         rearm_auto_attach(session_id, info["type"])
         handle_attach(target, session_id, params, client)
       end
+
+      client.on("Target.detachedFromTarget") do |params|
+        forget_session(params["targetId"], params["sessionId"])
+      end
     end
 
     def subscribe_target_created
@@ -257,6 +261,7 @@ def detach_untracked(session_id, parent)
     def detach_unless_manually_attached(target, session_id, parent)
       return if @manually_attached.delete(target.id)
 
+      forget_session(target.id, session_id)
       silent_detach(session_id, parent)
     end
 
@@ -273,6 +278,11 @@ def resume(session_id)
       @client.session(session_id).command("Runtime.runIfWaitingForDebugger", async: true)
     end
 
+    def forget_session(target_id, session_id)
+      target = find_by(target_id: target_id)&.targets&.[](target_id)
+      target.session_id = nil if target&.session_id == session_id
+    end
+
     def silent_detach(session_id, parent)
       Utils::Thread.spawn do
         @client.session(session_id).command("Runtime.runIfWaitingForDebugger")

sig/ferrum/context.rbs

@@ -39,7 +39,7 @@ module Ferrum
 
     def delete_target: (String target_id) -> Target?
 
-    def attach_target: (String target_id) -> true
+    def attach_target: (String target_id) -> Target
 
     def find_target: () { (Target) -> bool } -> Target?
 

sig/ferrum/contexts.rbs

@@ -59,6 +59,8 @@ module Ferrum
 
     def resume: (String session_id) -> void
 
+    def forget_session: (String target_id, String session_id) -> void
+
     def silent_detach: (String session_id, (Client | SessionClient) parent) -> Thread
 
     def discover: () -> void

spec/context_spec.rb

@@ -38,4 +38,23 @@
       same.close
     end
   end
+
+  describe "#attach_target" do
+    it "reuses the session of an already attached target", :launch do
+      browser = Ferrum::Browser.new(base_url: base_url,
+                                    extensions: [{ source: "window.runs = (window.runs || 0) + 1" }])
+      page = browser.create_page
+      page.go_to
+      target = browser.targets[page.target_id]
+      session_id = target.session_id
+
+      2.times { expect(browser.attach_target(target.id)).to eq(target) }
+      page.refresh
+
+      expect(target.session_id).to eq(session_id)
+      expect(page.evaluate("window.runs")).to eq(1)
+    ensure
+      browser&.quit
+    end
+  end
 end

spec/worker_spec.rb

@@ -86,12 +86,13 @@ def wait_for_target(&block)
     it "detaches from them so Chrome can terminate them" do
       page.go_to
       page.evaluate_async(%(navigator.serviceWorker.register("/sw.js").then(arguments[0])), 5)
-      wait_for_target(&:service_worker?)
+      target = wait_for_target(&:service_worker?)
 
       wait_for do
         browser.client.command("Target.getTargets")["targetInfos"]
                .select { |info| info["type"] == "service_worker" }.map { |info| info["attached"] }
       end.to eq([false])
+      expect(target.session_id).to be_nil
     end
 
     it "connects on demand through Context#attach_target, keeping it alive" do