Comment 1 Grant Gayed 2013-04-15 10:43:24 EDT

The patch makes perfect sense and works well, thanks for your contribution!  Fixed > 20130415, commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=61aaa5d345230600c799f6ea9d7e339f868270d4 .

Comment 2 Grant Gayed 2013-04-15 11:54:28 EDT

Note to self: This change will cause problems for clients doing the same wrong thing as IE.java was (disposing the OleEvent's argument Variants).  Document in porting guide.

Comment 3 Martin Kolb 2013-04-19 07:17:31 EDT

*** Bug 402453 has been marked as a duplicate of this bug. ***

Comment 4 Christof Engel 2013-06-11 05:55:42 EDT

This code snippet shows a WORKAROUND for the bugs mentioned above. To make use of the fixes you have to instantiate instances of BugFixedBrowser instead of the SWT Browser class.

///////////////// BEGIN snippet /////////////////////
package com.mycomp.mycontrol;

import java.lang.reflect.Field;

import org.eclipse.swt.SWT;
import org.eclipse.swt.browser.Browser;
import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.internal.ole.win32.IDispatch;
import org.eclipse.swt.internal.ole.win32.ITypeInfo;
import org.eclipse.swt.internal.ole.win32.IUnknown;
import org.eclipse.swt.internal.win32.OS;
import org.eclipse.swt.ole.win32.OleAutomation;
import org.eclipse.swt.ole.win32.OleControlSite;
import org.eclipse.swt.ole.win32.OleEvent;
import org.eclipse.swt.ole.win32.OleListener;
import org.eclipse.swt.ole.win32.Variant;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Event;
import org.eclipse.swt.widgets.Listener;

/**
 * Own Browser subclass to work around some huge memory leaks in SWT, and to avoid storing the navigation history on setUrl().
 * A patch for the memory leaks (occuring in the COM area) has been provided to the SWT team and is shipped with Eclipse 4.3.
 * For older releases (3.x, 4.2 as used by Design Studio), we can fix most of the issues reflectively in this class.
 */
@SuppressWarnings("restriction")
public class BugFixedBrowser extends Browser {

	private static final boolean DISABLE_SWT_PATCH = Boolean.getBoolean("com.mycomp.disableSWTPatch"); //$NON-NLS-1$

	private static final String PROPERTY_TYPE = "type"; //$NON-NLS-1$

	private static final String EVENT_MOUSEMOVE = "mousemove"; //$NON-NLS-1$

	private static final String PROPERTY_SCREENX = "screenX"; //$NON-NLS-1$

	private static final String PROPERTY_SCREENY = "screenY"; //$NON-NLS-1$

	static final String ABOUT_BLANK = "about:blank"; //$NON-NLS-1$

	static final int DocumentComplete = 0x103;

	/**
	 * Comment for <code>originalDomListener</code>
	 */
	protected OleListener originalDomListener;

	@SuppressWarnings("javadoc")
	protected final Browser browser;

	@SuppressWarnings("javadoc")
	protected int lastMouseMoveX, lastMouseMoveY;

	private BugFixingDocumentCompleteOleListener bugFixingListenerDocComplete;

	private class BugFixingDocumentCompleteOleListener implements OleListener {
		public BugFixingDocumentCompleteOleListener() {
		}

		@Override
		public void handleEvent(OleEvent e) {
			try {
				fixHandleEventDocumentCompleteWhenBrowserDisposed(e);

			} catch (Exception ex) {
				// nothing to do
			}
		}

		/*
		   IE.handleEvent => case DocumentComplete
		   if (browser.isDisposed()) return; << cleanup code at end of handleEvent is not run
		   
		   This only occurs when SWT widget is closed faster than DocumentComplete
		   is fired and should hopefully occur rarely.
		*/
		private void fixHandleEventDocumentCompleteWhenBrowserDisposed(OleEvent event) {
			if (event.type != DocumentComplete) {
				return; // should not occur because of registration
			}

			Object webBrowser;
			try {
				webBrowser = getWebBrowserField();

				OleAutomation auto = (OleAutomation) getFieldOfWebBrowser(webBrowser, "auto"); //$NON-NLS-1$ 
				if (auto != null) { // auto
					switch (event.type) {
						case DocumentComplete: {
							boolean performingInitialNavigate = ((Boolean) getFieldOfWebBrowser(webBrowser, "performingInitialNavigate")).booleanValue(); //$NON-NLS-1$
							if (performingInitialNavigate) {
								break;
							}

							Variant varResult = event.arguments[1];
							String url = varResult.getString();

							String html = (String) getFieldOfWebBrowser(webBrowser, "html"); //$NON-NLS-1$
							if (html != null && url.equals(ABOUT_BLANK)) {
								// nothing to do
							} else {
								if (BugFixedBrowser.this.browser.isDisposed()) {
									// in the SWT code there is a return statement which leads to a skipping of the following cleanup code
									// at the end of the handleEvent method:
									// --------------------------------------
									/*
									* Dispose all arguments passed in the OleEvent.  This must be
									* done to properly release any IDispatch reference that was
									* automatically addRef'ed when constructing the OleEvent.  
									*/
									Variant[] arguments = event.arguments;
									for (Variant argument : arguments) {
										argument.dispose();
									}
									// --------------------------------------
									return;
								}
							}
						}
							break;
					}
				}
			} catch (Exception e) {
				// Logger.logError("Problem fixing documentComplete bug", e); //$NON-NLS-1$
			}

		}

	}

	// https://bugs.eclipse.org/bugs/show_bug.cgi?id=405711
	final class BugFixingOleListener implements OleListener {
		@Override
		public void handleEvent(OleEvent e) {
			BugFixedBrowser.this.originalDomListener.handleEvent(e);
			Variant arg = e.arguments[0];
			OleAutomation event = arg.getAutomation();
			try {
				fixHandleDomEventReturnBug(event);

			} finally {
				fixOleEventSinkInvokeBug(e);

				event.dispose();
			}
		}

		// Fix for return statement in IE.handleDOMEvent skipping event.dispose at end of function
		private void fixHandleDomEventReturnBug(OleAutomation event) {
			int[] rgdispid = event.getIDsOfNames(new String[] {PROPERTY_TYPE});
			int dispIdMember = rgdispid[0];
			Variant pVarResult = event.getProperty(dispIdMember);
			String eventType = pVarResult.getString();
			pVarResult.dispose();
			if (eventType.equals(EVENT_MOUSEMOVE)) {
				Event newEvent = new Event();
				/*
				* The position of mouse events is received in screen-relative coordinates
				* in order to handle pages with frames, since frames express their event
				* coordinates relative to themselves rather than relative to their top-
				* level page.  Convert screen-relative coordinates to be browser-relative.
				*/
				rgdispid = event.getIDsOfNames(new String[] {PROPERTY_SCREENX});
				dispIdMember = rgdispid[0];
				pVarResult = event.getProperty(dispIdMember);
				int screenX = pVarResult.getInt();
				pVarResult.dispose();

				rgdispid = event.getIDsOfNames(new String[] {PROPERTY_SCREENY});
				dispIdMember = rgdispid[0];
				pVarResult = event.getProperty(dispIdMember);
				int screenY = pVarResult.getInt();
				pVarResult.dispose();

				Point position = new Point(screenX, screenY);
				position = BugFixedBrowser.this.browser.getDisplay().map(null, BugFixedBrowser.this.browser, position);
				newEvent.x = position.x;
				newEvent.y = position.y;
				/*
				* Feature in IE. Spurious and redundant mousemove
				* events are often received. The workaround is to not
				* fire MouseMove events whose x and y values match the
				* last MouseMove.
				*/
				if (newEvent.x == BugFixedBrowser.this.lastMouseMoveX && newEvent.y == BugFixedBrowser.this.lastMouseMoveY) {
					immutableDispose(event); // 1 extra dispose which was skipped in IE.handleDOMEvent 
				} else {
					BugFixedBrowser.this.lastMouseMoveX = newEvent.x;
					BugFixedBrowser.this.lastMouseMoveY = newEvent.y;
				}
			}
		}

		// fix for bug in OleEventSink.Invoke
		private void fixOleEventSinkInvokeBug(OleEvent e) {
			if (e != null && e.arguments != null) {
				for (Variant argument : e.arguments) {
					argument.dispose();
				}
			}
		}
	}

	// dispose event object without resetting its inner interface pointers; code is from OleAutomation.dispose
	static void immutableDispose(OleAutomation event) {
		Class<?> theClass;
		try {
			theClass = Class.forName("org.eclipse.swt.ole.win32.OleAutomation"); //$NON-NLS-1$
			Field field = theClass.getDeclaredField("objIDispatch"); //$NON-NLS-1$
			field.setAccessible(true);
			IDispatch objIDispatch = (IDispatch) field.get(event);
			if (objIDispatch != null) {
				objIDispatch.Release();
			}
			field = theClass.getDeclaredField("objITypeInfo"); //$NON-NLS-1$
			field.setAccessible(true);
			ITypeInfo objITypeInfo = (ITypeInfo) field.get(event);
			if (objITypeInfo != null) {
				objITypeInfo.Release();
			}
			field = theClass.getDeclaredField("objIUnknown"); //$NON-NLS-1$
			field.setAccessible(true);
			IUnknown objIUnknown = (IUnknown) field.get(event);
			if (objIUnknown != null) {
				objIUnknown.Release();
				OS.OleUninitialize();
			}
		} catch (Exception e) {
			// Logger.logError("Failed to dispose event object", e); //$NON-NLS-1$
		}
	}

	/**
	 * @param parent
	 */
	public BugFixedBrowser(Composite parent) {
		super(parent, SWT.NONE);
		this.browser = this;
		if (!DISABLE_SWT_PATCH) {
			try {
				// 1. DOM listener
				Field field = Class.forName("org.eclipse.swt.browser.IE").getDeclaredField("domListener"); //$NON-NLS-1$ //$NON-NLS-2$
				field.setAccessible(true);
				Object ie = getWebBrowserField();
				this.originalDomListener = (OleListener) field.get(ie);
				OleListener bugFixingListener = new BugFixingOleListener();
				field.set(ie, bugFixingListener);
				// ------------------------

				// 2. DocumentComplete listener
				field = Class.forName("org.eclipse.swt.browser.IE").getDeclaredField("site"); //$NON-NLS-1$ //$NON-NLS-2$
				field.setAccessible(true);
				OleControlSite site = (OleControlSite) field.get(ie);
				this.bugFixingListenerDocComplete = new BugFixingDocumentCompleteOleListener();
				site.addEventListener(DocumentComplete, this.bugFixingListenerDocComplete);

				Listener disposeListener = new Listener() {
					@Override
					public void handleEvent(Event e) {
						switch (e.type) {
							case SWT.Dispose: {
								removeEventListener();
								break;
							}
						}
					}
				};
				addListener(SWT.Dispose, disposeListener);
				// ------------------------

			} catch (Exception e) {
				// Logger.logError("Failed to apply SWT patch", e); //$NON-NLS-1$
			}
		}
	}

	void removeEventListener() {
		if (!DISABLE_SWT_PATCH) {
			try {
				// 1. DOM listener
				// Do NOT change value of org.eclipse.swt.browser.IE.domListener as this is necessary for removeEventListener calls. 
				// And better keep value of this.originalDomListener untouched as there might be some DOM event coming in after this code here has run.

				// 2. DocumentComplete listener
				Field field = Class.forName("org.eclipse.swt.browser.IE").getDeclaredField("site"); //$NON-NLS-1$ //$NON-NLS-2$
				Object ie = getWebBrowserField();
				field.setAccessible(true);
				OleControlSite site = (OleControlSite) field.get(ie);
				if (site != null) {
					site.removeEventListener(DocumentComplete, this.bugFixingListenerDocComplete);
				}
				this.bugFixingListenerDocComplete = null;

			} catch (Exception e) {
				// Logger.logError("Failed to remove event listener for SWT patch", e); //$NON-NLS-1$
			}
		}
	}

	@Override
	protected void checkSubclass() {
		// comment out check (otherwise SWT won't let us subclass)
	}


	///////////////////////////////////////////////////////////////////////////////////////////////

	Object getFieldOfWebBrowser(Object webBrowser, String fieldName) {
		try {
			Field field = webBrowser.getClass().getDeclaredField(fieldName);
			field.setAccessible(true);
			return field.get(webBrowser);
		} catch (Exception e) {
			// Logger.logError("Failed to access " + fieldName, e); //$NON-NLS-1$
			return null;
		}
	}

	Object getWebBrowserField() throws SecurityException, NoSuchFieldException, IllegalArgumentException, IllegalAccessException {
		Field field = Browser.class.getDeclaredField("webBrowser"); //$NON-NLS-1$
		field.setAccessible(true);
		return field.get(this);
	}

}
///////////////// EOF snippet /////////////////////

Comment 5 Christof Engel 2013-06-11 05:57:16 EDT

Created attachment 232224 [details]
Workaround sample code