💖 Thanks for opening this pull request! 💖

Semantic PR titles

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

• fix: don't overwrite prevent_default if default wasn't prevented
• feat: add app.isPackaged() method
• docs: app.isDefaultProtocolClient is now available on Linux

Commit signing

This repo enforces commit signatures for all incoming PRs.
To sign your commits, see GitHub's documentation on Telling Git about your signing key.

PR tips

Things that will help get your PR across the finish line:

• Follow the JavaScript, C++, and Python coding style.
• Run npm run lint locally to catch formatting errors earlier.
• Document any user-facing changes you've made following the documentation styleguide.
• Include tests when adding/changing behavior.
• Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

Unfortunately, this doesn't work properly because of what appears to me to be a bug in Chromium that should affect all <iframe>s that participate into an atomic move.

The "connected subframes count" is not maintained properly, which results in a crash in CheckConnectedSubframeCountIsConsistent.

The reason for this is that HTMLFrameOwnerElement::InsertedInto() increments the count of the <iframe> element and all its current ancestors, regardless of the element being moved, while HTMLFrameOwnerElement::RemovedFrom() only decrements the count of the <iframe> element itself and all the old ancestors of the element being moved.

In our case, if you perform an atomic move the <webview> element, HTMLFrameOwnerElement::InsertedInto() increments:

IFRAME
  #shadow-root(Open)
    WEBVIEW
      [... all the new ancestors of the WEBVIEW]

... while HTMLFrameOwnerElement::RemovedFrom() only decrements:

[... all the old ancestors of the WEBVIEW]

The following patch fixes this particular issue, and makes sense to me, but I will admit I do not know this code base at all, and I would appreciate help reporting / fixing this upstream:

diff --git a/third_party/blink/renderer/core/html/html_frame_owner_element.cc b/third_party/blink/renderer/core/html/htm
l_frame_owner_element.cc
index 8ce07b4801824..44d14a847182a 100644
--- a/third_party/blink/renderer/core/html/html_frame_owner_element.cc
+++ b/third_party/blink/renderer/core/html/html_frame_owner_element.cc
@@ -212,7 +212,7 @@ Node::InsertionNotificationRequest HTMLFrameOwnerElement::InsertedInto(
     // For the non-state-preserving atomic move case (i.e., when we're setting
     // up a full frame due to real insertion), this is done in
     // `HTMLFrameOwnerElement::SetContentFrame()` below.
-    for (ContainerNode* node = this; node;
+    for (ContainerNode* node = &insertion_point; node;
          node = node->ParentOrShadowHostNode()) {
       node->IncrementConnectedSubframeCount();
     }
@@ -232,10 +232,6 @@ void HTMLFrameOwnerElement::RemovedFrom(ContainerNode& insertion_point) {
   // but we still have to do (2) manually to maintain bookkeeping consistency
   // among the ancestor nodes.
   if (GetDocument().StatePreservingAtomicMoveInProgress() && ContentFrame()) {
-    // `this` is no longer connected, so we have to decrement our subframe count
-    // separately from our old ancestors's subframe count (i.e.,
-    // `insertion_point`).
-    DecrementConnectedSubframeCount();
     for (ContainerNode* node = &insertion_point; node;
          node = node->ParentOrShadowHostNode()) {
       node->DecrementConnectedSubframeCount();

@codebytere Thank you for taking a look. The code change is literally just the addition of the empty connectedMoveCallback method in lib/renderer/web-view/web-view-element.ts.

This allows moving the <webview> element with Element.moveBefore while persisting its state, as the browser calls connectedMoveCallback in this case instead of doing a disconnectedCallback / connectedCallback dance.

This is what the test itself asserts, that you can move a <webview> element without resetting its state, which we assert by making sure the page-title-updated event is not called again after the move. (Note that there is nothing specific to the page title here, any other event that triggers on the page being loaded inside the <webview> would work too).