Bug 1715036 - Always detach compositor. r=calu,owlish

Most of our JNI code either holds strong references to Java objects, i.e. the
Gecko Garbage Collector is responsible for clearning the object, or weak
references to Java code, i.e. the Java GC is responsible for clearing the
object.

Some objects, however, are special and need manual care. These objects
implement the `onWeakNonIntrusiveDetach` method, which is manually triggered by
calling `Detach`.

My understanding is that these objects need to be cleared from either the Java
side (when the GC destroys the GeckoSession) or from the C++ side, when the
window associated to a GeckoSession is closed.

This is done to avoid holding off to a window for longer than necessary, since
it's very expensive to do so.

The intermittent subject of this bug was caused by us not clearing the
Compositor object when the window closes on the Gecko side, letting the Java
side call `syncPauseCompositor` on a dead JNI object.

This is the before code of the Compositor's `onWeakNonIntrusiveDetach`:

```
  void OnWeakNonIntrusiveDetach(already_AddRefed<Runnable> aDisposer) {
    RefPtr<Runnable> disposer = aDisposer;
    if (RefPtr<nsThread> uiThread = GetAndroidUiThread()) {
      // ...
      uiThread->Dispatch(NS_NewRunnableFunction(
          "LayerViewSupport::OnWeakNonIntrusiveDetach",
          [compositor, disposer = std::move(disposer),
           results = &mCapturePixelsResults, window = mWindow]() mutable {
            if (auto accWindow = window.Access()) {
	      // ...
              compositor->OnCompositorDetached();
            }

            disposer->Run();
          }));
    }
```

As you can see from the above, the `OnCompositorDetached` method, which informs
the Java layer that the compositor should not be called anymore, was only
invoked when `window.Access()` returns successfully, i.e.  when we have a
window that is alive.

Normally, a Compositor object always has a window associated to it, as there's
a strong link between the GeckoSession and the Compositor object on the Java
side.

There are, however, some edge cases (mostly during shutdown) where the Java
code holds a reference to the Compositor but not the GeckoSession. In those
cases, we would fail the following check

```
if (auto accWindow = window.Access()) {
```

And fail to call `OnCompositorDetached`.

We don't really need the window to call `OnCompositorDetached`, as explained
above, so we can just move the call outside the `if` statement to fix this
problem.

Differential Revision: https://phabricator.services.mozilla.com/D130101
This commit is contained in:
Agi Sferro 2021-11-02 18:28:12 +00:00
Родитель 4d6d26edcd
Коммит 58998fbef1
1 изменённых файлов: 1 добавлений и 1 удалений

Просмотреть файл

@ -1004,9 +1004,9 @@ class LayerViewSupport final
}
results->pop();
}
compositor->OnCompositorDetached();
}
compositor->OnCompositorDetached();
disposer->Run();
}));
}