Skip to content

Commit

Permalink
Bug 1651705: Part 3 - Move nested classes out of nsWindow, switch ove…
Browse files Browse the repository at this point in the history
…r to NativeWeakPtr in those class definitions; r=geckoview-reviewers,agi

* Having `AndroidView` and `GeckoViewSupport` as nested classes inside of
  `nsWindow` make it impossible to forward declare them. We move those classes
  into their own headers. We also move `WindowEvent` into its own header.

* We remove the old `NativePtr` and `WindowPtr` implementations from `nsWindow`
  and convert the class definitions in this patch to use the new `NativeWeakPtr`.

* `GeckoViewSupport` had a unique quirk where it was owned by `nsWindow`
  instead of its Java counterpart. To make `GeckoViewSupport`'s ownership work
  like the other classes that use `NativeWeakPtr` (and to substantially simplify
  the implementation of `NativeWeakPtr` itself), I have reversed that: now
  `nsWindow` holds a `NativeWeakPtr` to `GeckoViewSupport`, while
  `GeckoViewSupport` is owned by its Java counterpart and holds a strong ref to
  the `nsWindow`.

* `GeckoViewSupport` no longer inherits from `SupportsWeakPtr`, since using it
  with `NativeWeakPtr` provides stronger and safer guarantees.

Differential Revision: https://phabricator.services.mozilla.com/D87362
  • Loading branch information
dblohm7 committed Sep 21, 2020
1 parent cf2a216 commit e6d5d8f
Show file tree
Hide file tree
Showing 6 changed files with 220 additions and 189 deletions.
35 changes: 35 additions & 0 deletions widget/android/AndroidView.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef mozilla_widget_AndroidView_h
#define mozilla_widget_AndroidView_h

#include "mozilla/widget/EventDispatcher.h"

namespace mozilla {
namespace widget {

class AndroidView final : public nsIAndroidView {
virtual ~AndroidView() {}

public:
const RefPtr<mozilla::widget::EventDispatcher> mEventDispatcher{
new mozilla::widget::EventDispatcher()};

AndroidView() {}

NS_DECL_ISUPPORTS
NS_DECL_NSIANDROIDVIEW

NS_FORWARD_NSIANDROIDEVENTDISPATCHER(mEventDispatcher->)

mozilla::java::GeckoBundle::GlobalRef mInitData;
};

} // namespace widget
} // namespace mozilla

#endif // mozilla_widget_AndroidView_h
98 changes: 98 additions & 0 deletions widget/android/GeckoViewSupport.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef mozilla_widget_GeckoViewSupport_h
#define mozilla_widget_GeckoViewSupport_h

#include "mozilla/java/GeckoResultWrappers.h"
#include "mozilla/java/GeckoSessionNatives.h"
#include "mozilla/widget/WindowEvent.h"

class nsWindow;

namespace mozilla {
namespace widget {

class GeckoViewSupport final
: public java::GeckoSession::Window::Natives<GeckoViewSupport> {
RefPtr<nsWindow> mWindow;

// We hold a WeakRef because we want to allow the
// GeckoSession.Window to be garbage collected.
// Callers need to create a LocalRef from this
// before calling methods.
java::GeckoSession::Window::WeakRef mGeckoViewWindow;

public:
typedef java::GeckoSession::Window::Natives<GeckoViewSupport> Base;

template <typename Functor>
static void OnNativeCall(Functor&& aCall) {
NS_DispatchToMainThread(new WindowEvent<Functor>(std::move(aCall)));
}

GeckoViewSupport(nsWindow* aWindow,
const java::GeckoSession::Window::LocalRef& aInstance,
nsPIDOMWindowOuter* aDOMWindow)
: mWindow(aWindow), mGeckoViewWindow(aInstance), mDOMWindow(aDOMWindow) {}

~GeckoViewSupport();

nsWindow* GetNsWindow() const { return mWindow; }

using Base::DisposeNative;

/**
* GeckoView methods
*/
private:
nsCOMPtr<nsPIDOMWindowOuter> mDOMWindow;
bool mIsReady{false};

public:
// Create and attach a window.
static void Open(const jni::Class::LocalRef& aCls,
java::GeckoSession::Window::Param aWindow,
jni::Object::Param aQueue, jni::Object::Param aCompositor,
jni::Object::Param aDispatcher,
jni::Object::Param aSessionAccessibility,
jni::Object::Param aInitData, jni::String::Param aId,
jni::String::Param aChromeURI, int32_t aScreenId,
bool aPrivateMode, bool aRemote);

// Close and destroy the nsWindow.
void Close();

// Transfer this nsWindow to new GeckoSession objects.
void Transfer(const java::GeckoSession::Window::LocalRef& inst,
jni::Object::Param aQueue, jni::Object::Param aCompositor,
jni::Object::Param aDispatcher,
jni::Object::Param aSessionAccessibility,
jni::Object::Param aInitData);

void AttachEditable(const java::GeckoSession::Window::LocalRef& inst,
jni::Object::Param aEditableParent);

void AttachAccessibility(const java::GeckoSession::Window::LocalRef& inst,
jni::Object::Param aSessionAccessibility);

void OnReady(jni::Object::Param aQueue = nullptr);

auto OnLoadRequest(mozilla::jni::String::Param aUri, int32_t aWindowType,
int32_t aFlags, mozilla::jni::String::Param aTriggeringUri,
bool aHasUserGesture, bool aIsTopLevel) const
-> java::GeckoResult::LocalRef;

void OnWeakNonIntrusiveDetach(already_AddRefed<Runnable> aDisposer) {
RefPtr<Runnable> disposer(aDisposer);
disposer->Run();
}
};

} // namespace widget
} // namespace mozilla

#endif // mozilla_widget_GeckoViewSupport_h
57 changes: 57 additions & 0 deletions widget/android/WindowEvent.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef mozilla_widget_WindowEvent_h
#define mozilla_widget_WindowEvent_h

#include "nsThreadUtils.h"
#include "mozilla/jni/Natives.h"

namespace mozilla {
namespace widget {

// An Event subclass that guards against stale events.
// (See the implmentation of mozilla::jni::detail::ProxyNativeCall for info
// about the default template parameters for this class)
template <typename Lambda, bool IsStatic = Lambda::isStatic,
typename InstanceType = typename Lambda::ThisArgType,
class Impl = typename Lambda::TargetClass>
class WindowEvent : public Runnable {
bool IsStaleCall() {
if (IsStatic) {
// Static calls are never stale.
return false;
}

return jni::NativePtrTraits<Impl>::IsStale(mInstance);
}

Lambda mLambda;
const InstanceType mInstance;

public:
WindowEvent(Lambda&& aLambda, InstanceType&& aInstance)
: Runnable("mozilla::widget::WindowEvent"),
mLambda(std::move(aLambda)),
mInstance(std::forward<InstanceType>(aInstance)) {}

explicit WindowEvent(Lambda&& aLambda)
: Runnable("mozilla::widget::WindowEvent"),
mLambda(std::move(aLambda)),
mInstance(mLambda.GetThisArg()) {}

NS_IMETHOD Run() override {
if (!IsStaleCall()) {
mLambda();
}
return NS_OK;
}
};

} // namespace widget
} // namespace mozilla

#endif // mozilla_widget_WindowEvent_h
3 changes: 3 additions & 0 deletions widget/android/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,11 @@ sources_from_WrapForJNI = sorted(
EXPORTS.mozilla.widget += [
'AndroidCompositorWidget.h',
'AndroidUiThread.h',
'AndroidView.h',
'EventDispatcher.h',
'GeckoViewSupport.h',
'nsWindow.h',
'WindowEvent.h',
]

EXPORTS.mozilla.java += [
Expand Down
46 changes: 2 additions & 44 deletions widget/android/nsWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ using mozilla::gfx::SurfaceFormat;
#include "AndroidBridge.h"
#include "AndroidBridgeUtilities.h"
#include "AndroidUiThread.h"
#include "AndroidView.h"
#include "GeckoEditableSupport.h"
#include "GeckoViewSupport.h"
#include "KeyEvent.h"
#include "MotionEvent.h"
#include "mozilla/java/EventDispatcherWrappers.h"
Expand Down Expand Up @@ -142,50 +144,6 @@ static const int32_t INPUT_RESULT_HANDLED =
static const int32_t INPUT_RESULT_HANDLED_CONTENT =
java::PanZoomController::INPUT_RESULT_HANDLED_CONTENT;

template <typename Lambda, bool IsStatic, typename InstanceType, class Impl>
class nsWindow::WindowEvent : public Runnable {
bool IsStaleCall() {
if (IsStatic) {
// Static calls are never stale.
return false;
}

JNIEnv* const env = mozilla::jni::GetEnvForThread();

const auto natives = reinterpret_cast<mozilla::WeakPtr<Impl>*>(
jni::GetNativeHandle(env, mInstance.Get()));
MOZ_CATCH_JNI_EXCEPTION(env);

// The call is stale if the nsWindow has been destroyed on the
// Gecko side, but the Java object is still attached to it through
// a weak pointer. Stale calls should be discarded. Note that it's
// an error if natives is nullptr here; we return false but the
// native call will throw an error.
return natives && !natives->get();
}

Lambda mLambda;
const InstanceType mInstance;

public:
WindowEvent(Lambda&& aLambda, InstanceType&& aInstance)
: Runnable("nsWindowEvent"),
mLambda(std::move(aLambda)),
mInstance(std::forward<InstanceType>(aInstance)) {}

explicit WindowEvent(Lambda&& aLambda)
: Runnable("nsWindowEvent"),
mLambda(std::move(aLambda)),
mInstance(mLambda.GetThisArg()) {}

NS_IMETHOD Run() override {
if (!IsStaleCall()) {
mLambda();
}
return NS_OK;
}
};

namespace {
template <class Instance, class Impl>
std::enable_if_t<jni::detail::NativePtrPicker<Impl>::value ==
Expand Down
Loading

0 comments on commit e6d5d8f

Please sign in to comment.