I should also note that the test passes fine without the --additional-drt-flag=--site-per-process flag. I found this when investigating my change that was reverted in https://chromium-review.googlesource.com/c/chromium/src/+/739882

It does seem that the test should pass.  I am guessing that the test harness might be stuck because it thinks that some frames are still loading (when in reality the child frame has simply been removed).

Given that this seems to be an issue unrelated to the reverted change (e.g. to https://chromium-review.googlesource.com/c/chromium/src/+/739882), it is probably fine to reland after tweaking the CL to add a test exception to third_party/WebKit/LayoutTests/FlagExpectations/site-per-process.

Without --site-per-process, the call to test.done() made by site-isolation-postmessage-cross-origin.html results in the following callstack (line numbers may be slightly off due to printf-style debugging statements added by me to the scripts):

    at completionCallback (http://127.0.0.1:8000/resources/testharnessreport.js:229:17)
    at http://127.0.0.1:8000/resources/testharness.js:2078:22
    at forEach (http://127.0.0.1:8000/resources/testharness.js:2772:26)
    at Tests.notify_complete (http://127.0.0.1:8000/resources/testharness.js:2075:9)
    at Tests.complete (http://127.0.0.1:8000/resources/testharness.js:2030:14)
    at Tests.notify_result (http://127.0.0.1:8000/resources/testharness.js:2010:22)
    at Tests.result (http://127.0.0.1:8000/resources/testharness.js:1997:14)
    at Test.done (http://127.0.0.1:8000/resources/testharness.js:1618:15)
    at Test.window.addEventListener.test.step_func.evt (http://127.0.0.1:8000/misc/site-isolation-postmessage-cross-origin.html:20:12)
    at Test.step (http://127.0.0.1:8000/resources/testharness.js:1491:25)

With --site-per-process, the difference is that we skip the call here:

    Tests.prototype.notify_result = function(test) {
        testRunner.logToStderr("Test.prototype.notify_result: 1");
        var this_obj = this;
        this.processing_callbacks = true;
        forEach(this.test_done_callbacks,
                function(callback)
                {
                    callback(test, this_obj);
                });
        this.processing_callbacks = false;
        testRunner.logToStderr("Test.prototype.notify_result: 2");
        if (this_obj.all_done()) {
            testRunner.logToStderr("Test.prototype.notify_result: 3");  <-- WE DON'T GET HERE WITH --site-per-process
            this_obj.complete();
        }
    };

Without --site-per-process Tests.all_done sees:

    this.phase = 3
    this.tests.length = 1
    test_environment_all_loaded = true
    this.num_pending = 0
    this.wait_for_finish = false
    this.processing_callbacks = false
    pending_remotes.some(...) = false


With --site-per-process Tests.all_done sees:

    this.phase = 3
    this.tests.length = 1
    test_environment_all_loaded = false <- difference here...
    this.num_pending = 0
    this.wait_for_finish = false
    this.processing_callbacks = false
    pending_remotes.some(...) = false

Without --site-per-process, the 'load' event (hooked up in testharness.js by WindowTestEnvironment via on_event(window, 'load', function() { ... ) is firing twice - once for each frame.

With --site-per-process, I see the 'load' event firing in the OOPIF process, but there is no 'load' event happening in the main process.

One way to get the missing load event to fire is to make the following change in HTMLFrameOwnerElement::DisconnectContentFrame():

--- a/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp
@@ -119,6 +119,8 @@ void HTMLFrameOwnerElement::DisconnectContentFrame() {
   if (Frame* frame = ContentFrame()) {
     frame->Detach(FrameDetachType::kRemove);
   }
+
+  GetDocument().CheckCompleted();
 }


I hope that is a reasonable change...  I'll put it together into a CL to see what the trybots say.

WIP CL @ https://chromium-review.googlesource.com/c/chromium/src/+/744985

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a950cd569f0def14619ecc9e4cd3738da61978f8

commit a950cd569f0def14619ecc9e4cd3738da61978f8
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Nov 03 16:14:00 2017

Consider firing a 'load' event after removing a still-loading frame.

Firing of a 'load' event in a parent can be postponed/suppressed while
a child frame is still loading - see AllDescendantsAreComplete function
that is consulted by Document::ShouldComplete method.  The problem
is that the result of AllDescendantsAreComplete can be impacted by
removal of a child frame, but the removal doesn't trigger a call to
Document::CheckCompleted - this can lead to no 'load' event being
raised at all.

Bug:  779433 
Change-Id: I003f8d8b4eeaab61e13b3c7d027ed8deeb84bf95
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Reviewed-on: https://chromium-review.googlesource.com/744985
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513795}
[add] https://crrev.com/a950cd569f0def14619ecc9e4cd3738da61978f8/third_party/WebKit/LayoutTests/http/tests/misc/load-event-after-removing-child-frame.html
[add] https://crrev.com/a950cd569f0def14619ecc9e4cd3738da61978f8/third_party/WebKit/LayoutTests/http/tests/misc/resources/subframe-for-load-event-after-removing-child-frame.html
[modify] https://crrev.com/a950cd569f0def14619ecc9e4cd3738da61978f8/third_party/WebKit/LayoutTests/http/tests/resources/feature-policy-permissions-test.js
[modify] https://crrev.com/a950cd569f0def14619ecc9e4cd3738da61978f8/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp