Only emit frameAttached when frame is committed

This fixes a problem where frameAttached is not emitted when navigating
a frame from an OOPIF back into the parent's frame process.

Change-Id: I2c2ce150e810bcb97c179d8ad5cbc940a0c0b4ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4744066
Auto-Submit: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1179345}

third_party/blink/renderer/core/frame/local_frame.cc

@@ -1715,21 +1715,21 @@ LocalFrame::LocalFrame(LocalFrameClient* client,
   DCHECK(ad_tracker_ ? RuntimeEnabledFeatures::AdTaggingEnabled()
                      : !RuntimeEnabledFeatures::AdTaggingEnabled());
 
-  absl::optional<AdScriptIdentifier> ad_script_on_stack;
   // See SubresourceFilterAgent::Initialize for why we don't set this here for
   // fenced frames.
   is_frame_created_by_ad_script_ =
       !IsMainFrame() && ad_tracker_ &&
-      ad_tracker_->IsAdScriptInStack(AdTracker::StackType::kBottomAndTop,
-                                     /*out_ad_script=*/&ad_script_on_stack);
+      ad_tracker_->IsAdScriptInStack(
+          AdTracker::StackType::kBottomAndTop,
+          /*out_ad_script=*/&ad_script_from_frame_creation_stack_);
 
   Initialize();
   // Now that we know whether the frame is provisional, inherit the probe
   // sink from parent if appropriate. See comment above for more details.
   if (!IsLocalRoot() && !IsProvisional()) {
     probe_sink_ = LocalFrameRoot().probe_sink_;
+    probe::FrameAttachedToParent(this, ad_script_from_frame_creation_stack_);
   }
-  probe::FrameAttachedToParent(this, ad_script_on_stack);
 }
 
 FrameScheduler* LocalFrame::GetFrameScheduler() {
@@ -2729,6 +2729,7 @@ bool LocalFrame::SwapIn() {
     } else {
       probe_sink_ = LocalFrameRoot().probe_sink_;
     }
+    probe::FrameAttachedToParent(this, ad_script_from_frame_creation_stack_);
   }
 
   // First, check if there's a previous main frame to be used for a main frame

third_party/blink/renderer/core/frame/local_frame.h

@@ -74,6 +74,7 @@
 #include "third_party/blink/renderer/core/dom/weak_identifier_map.h"
 #include "third_party/blink/renderer/core/editing/forward.h"
 #include "third_party/blink/renderer/core/editing/iterators/text_iterator_behavior.h"
+#include "third_party/blink/renderer/core/frame/ad_script_identifier.h"
 #include "third_party/blink/renderer/core/frame/frame.h"
 #include "third_party/blink/renderer/core/frame/frame_types.h"
 #include "third_party/blink/renderer/core/frame/local_frame_view.h"
@@ -1142,6 +1143,10 @@ class CORE_EXPORT LocalFrame final
   // SubresourceFilterAgent::Initialize.
   bool is_frame_created_by_ad_script_ = false;
 
+  // The identifier of the ad script at the time of frame creation. Kept to
+  // defer instrumentation probe call till the frame is committed.
+  absl::optional<AdScriptIdentifier> ad_script_from_frame_creation_stack_;
+
   bool evict_cached_session_storage_on_freeze_or_unload_ = false;
 
   // Indicate if the current document's color scheme was notified.

third_party/blink/web_tests/FlagExpectations/disable-site-isolation-trials

@@ -77,6 +77,7 @@ http/tests/inspector-protocol/page/frame-detached-oopif.js [ Skip ]
 http/tests/inspector-protocol/timeline/auction-worklet-frame.js [ Skip ]
 virtual/fledge/http/tests/inspector-protocol/timeline/auction-worklet-frame.js [ Skip ]
 http/tests/inspector-protocol/page/frame-started-stopped-loading.js [ Skip ]
+http/tests/inspector-protocol/page/frame-attached.js [ Skip ]
 
 # Fix to unblock wpt-importer
 # The paths for these tests' baselines (with an `-expected.{txt,png}` suffix) is

third_party/blink/web_tests/http/tests/inspector-protocol/page/frame-attached-expected.txt

@@ -0,0 +1,12 @@
+Tests Page.frameAttachedevents with different kinds of frame navigation
+Initial navigation
+Navigating same origin
+Navigating cross origin
+http://oopif-a.devtools.test:8000/inspector-protocol/resources/empty.html
+Navigating cross origin again
+http://oopif-b.devtools.test:8000/inspector-protocol/resources/empty.html
+Navigating back to in-process
+main target: Page.frameAttached
+Attaching a local frame
+main target: Page.frameAttached
+

third_party/blink/web_tests/http/tests/inspector-protocol/page/frame-attached.js

@@ -0,0 +1,60 @@
+(async function(testRunner) {
+  const {session, dp} = await testRunner.startHTML(
+      `<html><body><iframe id='frame'></iframe></body></html>`,
+      'Tests Page.frameAttachedevents with ' +
+      'different kinds of frame navigation');
+
+  function setLogFrameAttachedEvents(dp, prefix) {
+    dp.Page.enable();
+    dp.Page.onceFrameAttached(e => testRunner.log(`${prefix}: ${e.method}`));
+  }
+
+  setLogFrameAttachedEvents(dp, 'main target');
+  await dp.Target.setAutoAttach({autoAttach: true, flatten: true, waitForDebuggerOnStart: true});
+
+  const url1 = 'http://127.0.0.1:8000/inspector-protocol/resources/empty.html';
+  const url2 = `${url1}?foo=bar`;
+  const url3 = `http://oopif-a.devtools.test:8000/inspector-protocol/resources/empty.html`;
+  const url4 = `http://oopif-b.devtools.test:8000/inspector-protocol/resources/empty.html`;
+
+  testRunner.log(`Initial navigation`);
+  session.evaluate(`document.getElementById('frame').src = '${url1}'`);
+  await dp.Page.onceFrameStoppedLoading();
+
+  testRunner.log(`Navigating same origin`);
+  session.evaluate(`document.getElementById('frame').src = '${url2}'`);
+  await dp.Page.onceFrameStoppedLoading();
+
+  testRunner.log(`Navigating cross origin`);
+
+  session.evaluate(`document.getElementById('frame').src = '${url3}'`);
+
+  let {params} = await dp.Target.onceAttachedToTarget();
+  const session2 = session.createChild(params.sessionId);
+  const dp2 = session2.protocol;
+  setLogFrameAttachedEvents(dp2, 'child target');
+  dp2.Runtime.runIfWaitingForDebugger();
+
+  await dp2.Page.onceDomContentEventFired();
+  testRunner.log(await session2.evaluate(`location.href`));
+
+  testRunner.log(`Navigating cross origin again`);
+  session.evaluate(`document.getElementById('frame').src = '${url4}'`);
+
+  await dp2.Page.onceDomContentEventFired();
+  testRunner.log(await session2.evaluate(`location.href`));
+
+  testRunner.log(`Navigating back to in-process`);
+  session.evaluate(`document.getElementById('frame').src = '${url1}'`);
+  await dp.Page.onceFrameStoppedLoading();
+
+  testRunner.log(`Attaching a local frame`);
+  session.evaluate(`(function() {
+    const frame = document.createElement('iframe');
+    frame.src = '${url1}';
+    document.body.appendChild(frame);
+  })()`);
+  await dp.Page.onceFrameAttached();
+
+  testRunner.completeTest();
+})