Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8088420: JavaFX WebView memory leak via EventListener
Reviewed-by: kcr, arapte
  • Loading branch information
Jay Bhaskar authored and kevinrushforth committed May 30, 2022
1 parent d677003 commit f534850
Show file tree
Hide file tree
Showing 11 changed files with 1,506 additions and 6 deletions.
Expand Up @@ -82,6 +82,7 @@ add_definitions(-DSTATICALLY_LINKED_WITH_WTF)
list(APPEND WebCore_PRIVATE_FRAMEWORK_HEADERS
bindings/java/JavaDOMUtils.h
bindings/java/JavaEventListener.h
bindings/java/EventListenerManager.h
bindings/java/JavaNodeFilterCondition.h
bridge/jni/jsc/BridgeUtils.h
dom/DOMStringList.h
Expand Down
Expand Up @@ -109,6 +109,7 @@ platform/network/java/URLLoader.cpp

bindings/java/JavaDOMUtils.cpp
bindings/java/JavaEventListener.cpp
bindings/java/EventListenerManager.cpp

page/java/DragControllerJava.cpp
page/java/EventHandlerJava.cpp
@@ -0,0 +1,101 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

#include "EventListenerManager.h"
#include "JavaEventListener.h"
#include "DOMWindow.h"

namespace WebCore {

EventListenerManager& EventListenerManager::get_instance()
{
static NeverDestroyed<EventListenerManager> sharedManager;
return sharedManager;
}

void EventListenerManager::registerListener(JavaEventListener *listener, const JLObject &listenerObj)
{
ListenerJObjectWrapper *temp_ref = new ListenerJObjectWrapper(listenerObj);
std::pair<JavaEventListener*, ListenerJObjectWrapper*> entry{ listener, temp_ref };
listenerJObjectMap.insert(entry);
}

void EventListenerManager::unregisterListener(JavaEventListener *listener)
{
std::map<JavaEventListener*, ListenerJObjectWrapper*>::iterator it;
it = listenerJObjectMap.find(listener);

if (it != listenerJObjectMap.end()) {
if (it->second && it->second->use_count() == 1) {
delete it->second;
it->second = nullptr;
listenerJObjectMap.erase(it); // remove from list
}
else if (it->second && it->second->use_count() > 1)
it->second->dref();
}
}

JGObject EventListenerManager::getListenerJObject(JavaEventListener *listener)
{
std::map<JavaEventListener*, ListenerJObjectWrapper*>::iterator it;
it = listenerJObjectMap.find(listener);
if (it != listenerJObjectMap.end())
return it->second->getListenerJObject();

return nullptr;
}

void EventListenerManager::registerDOMWindow(DOMWindow* window, JavaEventListener *listener)
{
std::map<JavaEventListener*, ListenerJObjectWrapper*>::iterator it;
it = listenerJObjectMap.find(listener);
if (it != listenerJObjectMap.end())
it->second->ref();

std::pair<JavaEventListener*, DOMWindow*> entry{ listener, window};
listenerDOMWindowMultiMap.insert(entry);
}

void EventListenerManager::unregisterDOMWindow(DOMWindow* window)
{
std::multimap<JavaEventListener*, DOMWindow*>::iterator win_it;
for (win_it = listenerDOMWindowMultiMap.begin(); win_it != listenerDOMWindowMultiMap.end();) {
// de register associated event listeners with window
// and remove the entry from the map
if (window == win_it->second) {
unregisterListener(win_it->first);

std::multimap<JavaEventListener*, DOMWindow*>::iterator tmp_it;
tmp_it = win_it;
++win_it;
listenerDOMWindowMultiMap.erase(tmp_it);
} else {
++win_it;
}
}
}

} // namespace WebCore
@@ -0,0 +1,85 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

#pragma once

#ifndef EVENT_LISTENER_MANAGER_H
#define EVENT_LISTENER_MANAGER_H


#include "config.h"

#include <map>
#include <wtf/NeverDestroyed.h>
#include <iterator>
#include <wtf/java/JavaRef.h>
#include <jni.h>

namespace WebCore {

class DOMWindow;
class JavaEventListener;


class ListenerJObjectWrapper {
JGObject listenerObj;
unsigned int ref_count = 0;
public:
ListenerJObjectWrapper(const JLObject& listenerObj) {
this->listenerObj = listenerObj;
}

~ListenerJObjectWrapper() {
listenerObj.clear();
}
JGObject getListenerJObject() { return listenerObj; }
void ref() { ++ref_count; }
void dref() { --ref_count; }
unsigned int use_count() { return ref_count;}
};

class EventListenerManager {
EventListenerManager() = default;
WTF_MAKE_NONCOPYABLE(EventListenerManager);

std::map<JavaEventListener*, ListenerJObjectWrapper*> listenerJObjectMap;
std::multimap<JavaEventListener*, DOMWindow*> listenerDOMWindowMultiMap;

friend class NeverDestroyed<EventListenerManager>;

public:
static EventListenerManager& get_instance();

void registerListener(JavaEventListener *listener, const JLObject &listenerJObj);
void unregisterListener(JavaEventListener *listener) ;
JGObject getListenerJObject(JavaEventListener *listener);

void registerDOMWindow(DOMWindow*, JavaEventListener *listener);
void unregisterDOMWindow(DOMWindow*);
};

} // namespace WebCore

#endif // EVENT_LISTENER_MANAGER_H
Expand Up @@ -47,7 +47,7 @@ bool JavaEventListener::operator==(const EventListener& other) const
const JavaEventListener* jother = other.isJavaEventListener()
? static_cast<const JavaEventListener*>(&other)
: nullptr;
return jother && isJavaEquals(m_joListener, jother->m_joListener);
return this == jother;
}

void JavaEventListener::handleEvent(ScriptExecutionContext& context, Event& event)
Expand All @@ -65,7 +65,7 @@ void JavaEventListener::handleEvent(ScriptExecutionContext& context, Event& even

event.ref();
env->CallVoidMethod(
m_joListener,
EventListenerManager::get_instance().getListenerJObject(this),
midFwkHandleEvent,
ptr_to_jlong(&event));

Expand Down
Expand Up @@ -27,6 +27,7 @@

#include "Event.h"
#include "EventListener.h"
#include "EventListenerManager.h"
#include "Node.h"

#include <wtf/Vector.h>
Expand All @@ -38,17 +39,15 @@ class JavaEventListener final : public EventListener {
public:
JavaEventListener(const JLObject &listener)
: EventListener(NativeEventListenerType)
, m_joListener(listener)
{
relaxAdoptionRequirement();
EventListenerManager::get_instance().registerListener(this, listener);
}

~JavaEventListener() override;
virtual ~JavaEventListener() override;

bool operator == (const EventListener&) const override;
void handleEvent(ScriptExecutionContext& context, Event& event) override;

JGObject m_joListener;
static ScriptExecutionContext* scriptExecutionContext();
bool isJavaEventListener() const override { return true; }
private:
Expand Down
Expand Up @@ -56,6 +56,11 @@
#include <wtf/StdLibExtras.h>
#include <wtf/Vector.h>

#if PLATFORM(JAVA)
#include "EventListenerManager.h"
#include "JavaEventListener.h"
#endif

namespace WebCore {

WTF_MAKE_ISO_ALLOCATED_IMPL(EventTarget);
Expand Down Expand Up @@ -149,6 +154,9 @@ bool EventTarget::removeEventListener(const AtomString& eventType, EventListener
InspectorInstrumentation::willRemoveEventListener(*this, eventType, listener, options.capture);

if (data->eventListenerMap.remove(eventType, listener, options.capture)) {
#if PLATFORM(JAVA)
EventListenerManager::get_instance().unregisterListener(static_cast<JavaEventListener *> (&listener));
#endif
if (eventNames().isWheelEventType(eventType))
invalidateEventListenerRegions();

Expand Down
Expand Up @@ -90,6 +90,11 @@
#include "ContentChangeObserver.h"
#endif

#if PLATFORM(JAVA)
#include "JavaEventListener.h"
#include "EventListenerManager.h"
#endif

namespace WebCore {

WTF_MAKE_ISO_ALLOCATED_IMPL(Node);
Expand Down Expand Up @@ -2141,6 +2146,10 @@ static inline bool tryAddEventListener(Node* targetNode, const AtomString& event
if (!targetNode->EventTarget::addEventListener(eventType, listener.copyRef(), options))
return false;

#if PLATFORM(JAVA)
EventListenerManager::get_instance().registerDOMWindow(targetNode->document().domWindow(),
static_cast<JavaEventListener *> (&listener.copyRef().get()));
#endif
targetNode->document().addListenerTypeIfNeeded(eventType);
if (eventNames().isWheelEventType(eventType))
targetNode->document().didAddWheelEventHandler(*targetNode);
Expand Down
Expand Up @@ -149,6 +149,10 @@
#include "PointerLockController.h"
#endif

#if PLATFORM(JAVA)
#include "EventListenerManager.h"
#endif

namespace WebCore {
using namespace Inspector;

Expand Down Expand Up @@ -443,6 +447,10 @@ DOMWindow::~DOMWindow()
#endif

removeLanguageChangeObserver(this);

#if PLATFORM(JAVA)
EventListenerManager::get_instance().unregisterDOMWindow(this);
#endif
}

RefPtr<MediaQueryList> DOMWindow::matchMedia(const String& media)
Expand Down

1 comment on commit f534850

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.