/hg/icedtea-web: Stabilize plugin initialization
Omair Majid
omajid at redhat.com
Wed Nov 3 09:52:27 PDT 2010
On 11/03/2010 12:46 PM, Deepak Bhole wrote:
> * Omair Majid<omajid at redhat.com> [2010-11-03 12:32]:
>> Hi,
>>
>> On 10/26/2010 05:50 PM, dbhole at icedtea.classpath.org wrote:
>>> changeset d4a914a000e3 in /hg/icedtea-web
>>> details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=d4a914a000e3
>>> author: Deepak Bhole<dbhole at redhat.com>
>>> date: Tue Oct 26 17:49:57 2010 -0700
>>>
>>> Stabilize plugin initialization
>>> - Fixed frame pop-up issue when tab is closed early
>>> - Fixed 100% CPU load when too many applets load in parallel
>>> - Fixed message queue processing to prioritize destroy first
>>>
>>
>> It appears that this commit broke nettbank:
>> https://www.portalbank.no/1100/. With this changeset, I can no
>> longer type in the text field :(
>>
>
> Hmm, did you bisect? The change is unrelated to keyboard input -- it
> only re-organizes the sequence of initialization events. Once the applet
> is running, it should be the same as before.
>
Yup, I used hg bisect. Then I manually confirmed. I was quite surprised
too as I dont see how it could change the applet's behaviour after it
starts. Infact, my first suspect was my own AppContext patch. So please
confirm this yourself. On the other hand, I recall that there was this
typing backwards bug once in firefox which the plugin could trigger -
maybe this is something similar? I just wanted to point out the
regression to make sure we didnt get surprises close to release time.
Thanks,
Omair
>>>
>>> diffstat:
>>>
>>> 3 files changed, 264 insertions(+), 98 deletions(-)
>>> ChangeLog | 25
>>> plugin/icedteanp/java/sun/applet/PluginAppletViewer.java | 300 +++++++----
>>> plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java | 37 +
>>>
>>> diffs (truncated from 635 to 500 lines):
>>>
>>> diff -r dfd749c077c3 -r d4a914a000e3 ChangeLog
>>> --- a/ChangeLog Tue Oct 26 12:01:22 2010 -0400
>>> +++ b/ChangeLog Tue Oct 26 17:49:57 2010 -0700
>>> @@ -1,3 +1,28 @@ 2010-10-26 Andrew Su<asu at redhat.com>
>>> +2010-10-26 Deepak Bhole<dbhole at redhat.com>
>>> +
>>> + * plugin/icedteanp/java/sun/applet/PluginAppletViewer.java:
>>> + Replace all status.put calls with calls to updateStatus().
>>> + (createPanel): Create a frame with a 0 handle. Use the new
>>> + waitForAppletInit function to wait until applet is ready.
>>> + (reFrame): Re-order code so that the panel is never parentless.
>>> + (handleMessage): Re-wrote message processing to handle destroy calls
>>> + correctly, checking for them more often to prevent a frame from popping up
>>> + if the tab/page is closed before loading finishes. Decode special
>>> + characters in the message.
>>> + (updateStatus): New function. Updates the status for the given instance if
>>> + applicable.
>>> + (destroyApplet): New function. Destroys a given applet and frees related
>>> + resources.
>>> + (waitForAppletInit): New function. Blocks until applet is initialized.
>>> + (parse): Remove part that decoded the params. Decoding is now done earlier
>>> + in handleMessage().
>>> + * plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java:
>>> + (getPriorityStrIfPriority): Mark destroy messages as priority.
>>> + (bringPriorityMessagesToFront): Scans the queue for priority messages and
>>> + brings them to the front.
>>> + (run): If the queue is not empty and there are no workers left, run
>>> + bringPriorityMessagesToFront() and retry.
>>> +
>>> 2010-10-26 Andrew Su<asu at redhat.com>
>>>
>>> * Makefile.am: Split rm -rf into rm -f and rmdir for launcher
>>> diff -r dfd749c077c3 -r d4a914a000e3 plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
>>> --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Tue Oct 26 12:01:22 2010 -0400
>>> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Tue Oct 26 17:49:57 2010 -0700
>>> @@ -136,12 +136,10 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> }
>>> }
>>> });
>>> +
>>> + // create the frame.
>>> + PluginAppletViewer.reFrame(null, identifier, System.out, 0, panel);
>>>
>>> -
>>> -
>>> - // create the frame.
>>> - PluginAppletViewer.reFrame(null, identifier, System.out, handle, panel);
>>> -
>>> panel.init();
>>>
>>> // Start the applet
>>> @@ -179,15 +177,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>>
>>> // Wait for the panel to initialize
>>> // (happens in a separate thread)
>>> - while (panel.getApplet() == null&&
>>> - ((NetxPanel) panel).isAlive()) {
>>> - try {
>>> - Thread.sleep(50);
>>> - PluginDebug.debug("Waiting for applet to initialize...");
>>> - } catch (InterruptedException ie) {
>>> - // just wait
>>> - }
>>> - }
>>> + PluginAppletViewer.waitForAppletInit((NetxPanel) panel);
>>>
>>> a = panel.getApplet();
>>>
>>> @@ -317,7 +307,17 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> */
>>> private static String defaultSaveFile = "Applet.ser";
>>>
>>> - private static enum PAV_INIT_STATUS {PRE_INIT, IN_INIT, INIT_COMPLETE, INACTIVE};
>>> +
>>> + /**
>>> + * Enumerates the current status of an applet
>>> + *
>>> + * PRE_INIT -> Parsing and initialization phase
>>> + * INIT_COMPLETE -> Initialization complete, reframe pending
>>> + * REFRAME_COMPLETE -> Reframe complete, applet is initialized and usable by the user
>>> + * INACTIVE -> Browser has directed that the applet be destroyed (this state is non-overridable except by DESTROYED)
>>> + * DESTROYED -> Applet has been destroyed
>>> + */
>>> + private static enum PAV_INIT_STATUS {PRE_INIT, INIT_COMPLETE, REFRAME_COMPLETE, INACTIVE, DESTROYED};
>>>
>>> /**
>>> * The panel in which the applet is being displayed.
>>> @@ -350,7 +350,6 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>>
>>> private static HashMap<Integer, PAV_INIT_STATUS> status =
>>> new HashMap<Integer,PAV_INIT_STATUS>();
>>> -
>>>
>>> private long handle = 0;
>>> private WindowListener windowEventListener = null;
>>> @@ -380,16 +379,20 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> return;
>>>
>>> PluginAppletViewer newFrame = new PluginAppletViewer(handle, identifier, statusMsgStream, panel);
>>> -
>>> +
>>> if (oldFrame != null) {
>>> applets.remove(oldFrame.identifier);
>>> oldFrame.removeWindowListener(oldFrame.windowEventListener);
>>> panel.removeAppletListener(oldFrame.appletEventListener);
>>> +
>>> + // Add first, remove later
>>> + newFrame.add("Center", panel);
>>> oldFrame.remove(panel);
>>> oldFrame.dispose();
>>> + } else {
>>> + newFrame.add("Center", panel);
>>> }
>>>
>>> - newFrame.add("Center", panel);
>>> newFrame.pack();
>>>
>>> newFrame.appletEventListener = new AppletEventListener(newFrame, newFrame);
>>> @@ -397,11 +400,6 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>>
>>> applets.put(identifier, newFrame);
>>>
>>> - // dispose oldframe if necessary
>>> - if (oldFrame != null) {
>>> - oldFrame.dispose();
>>> - }
>>> -
>>> PluginDebug.debug(panel + " reframed");
>>> }
>>>
>>> @@ -449,7 +447,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> this.frame = frame;
>>> this.appletViewer = appletViewer;
>>> }
>>> -
>>> +
>>> public void appletStateChanged(AppletEvent evt)
>>> {
>>> AppletPanel src = (AppletPanel)evt.getSource();
>>> @@ -483,9 +481,9 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> AppletPanel.changeFrameAppContext(frame, SunToolkit.targetToAppContext(a));
>>> else
>>> AppletPanel.changeFrameAppContext(frame, AppContext.getAppContext());
>>> -
>>> - status.put(appletViewer.identifier, PAV_INIT_STATUS.INIT_COMPLETE);
>>> -
>>> +
>>> + updateStatus(appletViewer.identifier, PAV_INIT_STATUS.INIT_COMPLETE);
>>> +
>>> break;
>>> }
>>> }
>>> @@ -510,7 +508,13 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>>
>>> try {
>>> if (message.startsWith("handle")) {
>>> -
>>> +
>>> + // If there is a key for this status, it means it
>>> + // was either initialized before, or destroy has been
>>> + // processed. Stop moving further.
>>> + if (updateStatus(identifier, PAV_INIT_STATUS.PRE_INIT) != null)
>>> + return;
>>> +
>>> // Extract the information from the message
>>> String[] msgParts = new String[4];
>>> for (int i=0; i< 3; i++) {
>>> @@ -519,32 +523,38 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> msgParts[i] = message.substring(spaceLocation + 1, nextSpaceLocation);
>>> message = message.substring(nextSpaceLocation + 1);
>>> }
>>> -
>>> +
>>> long handle = Long.parseLong(msgParts[0]);
>>> String width = msgParts[1];
>>> String height = msgParts[2];
>>> -
>>> +
>>> int spaceLocation = message.indexOf(' ', "tag".length()+1);
>>> String documentBase =
>>> UrlUtil.decode(message.substring("tag".length() + 1, spaceLocation));
>>> - String tag = message.substring(spaceLocation+1);
>>> + String tag = message.substring(spaceLocation+1);
>>> +
>>> + // Decode the tag
>>> + tag = tag.replace(">", ">");
>>> + tag = tag.replace("<", "<");
>>> + tag = tag.replace("&", "&");
>>> + tag = tag.replace(" ", "\n");
>>> + tag = tag.replace(" ", "\r");
>>> + tag = tag.replace(""", "\"");
>>>
>>> PluginDebug.debug ("Handle = " + handle + "\n" +
>>> "Width = " + width + "\n" +
>>> "Height = " + height + "\n" +
>>> "DocumentBase = " + documentBase + "\n" +
>>> "Tag = " + tag);
>>> -
>>> - status.put(identifier, PAV_INIT_STATUS.PRE_INIT);
>>> +
>>> PluginAppletViewer.parse
>>> (identifier, handle, width, height,
>>> new StringReader(tag),
>>> new URL(documentBase));
>>> -
>>>
>>> int maxWait = APPLET_TIMEOUT; // wait for applet to fully load
>>> int wait = 0;
>>> - while (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE)&&
>>> + while ( !applets.containsKey(identifier)&& // Map is populated only by reFrame
>>> (wait< maxWait)) {
>>>
>>> try {
>>> @@ -555,9 +565,44 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> }
>>> }
>>>
>>> - if (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE))
>>> + // If wait exceeded maxWait, we timed out. Throw an exception
>>> + if (wait>= maxWait)
>>> throw new Exception("Applet initialization timeout");
>>>
>>> + PluginAppletViewer oldFrame = applets.get(identifier);
>>> +
>>> + // We should not try to destroy an applet during
>>> + // initialization. It may cause an inconsistent state,
>>> + // which would bad if it's a trusted applet that
>>> + // read/writes to files
>>> + waitForAppletInit((NetxPanel) applets.get(identifier).panel);
>>> +
>>> + // Should we proceed with reframing?
>>> + if (status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE)) {
>>> + destroyApplet(identifier);
>>> + return;
>>> + }
>>> +
>>> + // Proceed with re-framing
>>> + reFrame(oldFrame, identifier, System.out, handle, oldFrame.panel);
>>> +
>>> + // There is a slight chance that destroy can happen
>>> + // between the above and below line
>>> + if (updateStatus(identifier, PAV_INIT_STATUS.REFRAME_COMPLETE).equals(PAV_INIT_STATUS.INACTIVE)) {
>>> + destroyApplet(identifier);
>>> + }
>>> +
>>> + } else if (message.startsWith("destroy")) {
>>> +
>>> + // Set it inactive, and try to do cleanup is applicable
>>> + PAV_INIT_STATUS previousStatus = updateStatus(identifier, PAV_INIT_STATUS.INACTIVE);
>>> + PluginDebug.debug("Destroy status set for " + identifier);
>>> +
>>> + if (previousStatus != null&&
>>> + previousStatus.equals(PAV_INIT_STATUS.REFRAME_COMPLETE)) {
>>> + destroyApplet(identifier);
>>> + }
>>> +
>>> } else {
>>> PluginDebug.debug ("Handling message: " + message + " instance " + identifier + " " + Thread.currentThread());
>>>
>>> @@ -568,7 +613,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> status.get(identifier).equals(PAV_INIT_STATUS.PRE_INIT)
>>> )
>>> );
>>> -
>>> +
>>> // don't bother processing further for inactive applets
>>> if (status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE))
>>> return;
>>> @@ -580,31 +625,131 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> e.printStackTrace();
>>>
>>> // If an exception happened during pre-init, we need to update status
>>> - if (status.get(identifier).equals(PAV_INIT_STATUS.PRE_INIT))
>>> - status.put(identifier, PAV_INIT_STATUS.INACTIVE);
>>> + updateStatus(identifier, PAV_INIT_STATUS.INACTIVE);
>>>
>>> throw new RuntimeException("Failed to handle message: " +
>>> message + " for instance " + identifier, e);
>>> }
>>> }
>>> -
>>> +
>>> + /**
>>> + * Sets the status unless an overriding status is set (e.g. if
>>> + * status is DESTROYED, it may not be overridden).
>>> + *
>>> + * @param identifier The identifier for which the status is to be set
>>> + * @param status The status to switch to
>>> + * @return The previous status
>>> + */
>>> + private static synchronized PAV_INIT_STATUS updateStatus(int identifier, PAV_INIT_STATUS newStatus) {
>>> +
>>> + PAV_INIT_STATUS prev = status.get(identifier);
>>> +
>>> + // If the status is set
>>> + if (status.containsKey(identifier)) {
>>> +
>>> + // Nothing may override destroyed status
>>> + if (status.get(identifier).equals(PAV_INIT_STATUS.DESTROYED)) {
>>> + return prev;
>>> + }
>>> +
>>> + // If status is inactive, only DESTROYED may override it
>>> + if (status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE)) {
>>> + if (!newStatus.equals(PAV_INIT_STATUS.DESTROYED)) {
>>> + return prev;
>>> + }
>>> + }
>>> + }
>>> +
>>> + // Else set to given status
>>> + status.put(identifier, newStatus);
>>> +
>>> + return prev;
>>> + }
>>> +
>>> + /**
>>> + * Destroys the given applet instance.
>>> + *
>>> + * This function may be called multiple times without problems.
>>> + * It does a synchronized check on the status and will only
>>> + * attempt to destroy the applet if not previously destroyed.
>>> + *
>>> + * @param identifier The instance which is to be destroyed
>>> + */
>>> +
>>> + private static synchronized void destroyApplet(int identifier) {
>>> +
>>> + PluginDebug.debug("DestroyApplet called for " + identifier);
>>> +
>>> + PAV_INIT_STATUS prev = updateStatus(identifier, PAV_INIT_STATUS.DESTROYED);
>>> +
>>> + // If already destroyed, return
>>> + if (prev.equals(PAV_INIT_STATUS.DESTROYED)) {
>>> + PluginDebug.debug(identifier + " already destroyed. Returning.");
>>> + return;
>>> + }
>>> +
>>> + // If already disposed, return
>>> + if (applets.get(identifier).panel.applet == null) {
>>> + // Try to still dispose the panel itself -- no harm done with double dispose
>>> + applets.get(identifier).dispose();
>>> +
>>> + PluginDebug.debug(identifier + " inactive. Returning.");
>>> + return;
>>> + }
>>> +
>>> + PluginDebug.debug("Attempting to destroy " + identifier);
>>> +
>>> + final int fIdentifier = identifier;
>>> + SwingUtilities.invokeLater(new Runnable() {
>>> + public void run() {
>>> + applets.get(fIdentifier).appletClose();
>>> + }
>>> + });
>>> +
>>> + PluginDebug.debug(identifier + " destroyed");
>>> + }
>>> +
>>> + /**
>>> + * Function to block until applet initialization is complete
>>> + *
>>> + * @param identifier The instance to wait for
>>> + */
>>> + public static void waitForAppletInit(NetxPanel panel) {
>>> +
>>> + int waitTime = 0;
>>> +
>>> + // Wait till initialization finishes
>>> + while (panel.getApplet() == null&&
>>> + panel.isAlive()&&
>>> + waitTime< APPLET_TIMEOUT) {
>>> + try {
>>> + if (waitTime%500 == 0)
>>> + PluginDebug.debug("Waiting for applet panel " + panel + " to initialize...");
>>> +
>>> + Thread.sleep(waitTime += 50);
>>> + } catch (InterruptedException ie) {
>>> + // just wait
>>> + }
>>> + }
>>> +
>>> + PluginDebug.debug("Applet panel " + panel + " initialized");
>>> + }
>>> +
>>> public void handleMessage(int reference, String message)
>>> {
>>> if (message.startsWith("width")) {
>>>
>>> // Wait for panel to come alive
>>> int maxWait = APPLET_TIMEOUT; // wait for panel to come alive
>>> - int wait = 0;
>>> + int wait = 0;
>>> while (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE)&& wait< maxWait) {
>>> -
>>> - try {
>>> - Thread.sleep(50);
>>> - wait += 50;
>>> - } catch (InterruptedException ie) {
>>> - // just wait
>>> - }
>>> - }
>>> -
>>> + try {
>>> + Thread.sleep(50);
>>> + wait += 50;
>>> + } catch (InterruptedException ie) {
>>> + // just wait
>>> + }
>>> + }
>>>
>>> // 0 => width, 1=> width_value, 2 => height, 3=> height_value
>>> String[] dimMsg = message.split(" ");
>>> @@ -636,7 +781,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>>
>>> panel.setSize(width, height);
>>> panel.validate();
>>> -
>>> +
>>> panel.applet.resize(width, height);
>>> panel.applet.validate();
>>> }
>>> @@ -649,9 +794,6 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> e.printStackTrace();
>>> }
>>>
>>> - } else if (message.startsWith("destroy")) {
>>> - dispose();
>>> - status.put(identifier, PAV_INIT_STATUS.INACTIVE);
>>> } else if (message.startsWith("GetJavaObject")) {
>>>
>>> // FIXME: how do we determine what security context this
>>> @@ -672,15 +814,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>>
>>> // Wait for the panel to initialize
>>> // (happens in a separate thread)
>>> - while (panel.getApplet() == null&&
>>> - ((NetxPanel) panel).isAlive()) {
>>> - try {
>>> - Thread.sleep(50);
>>> - PluginDebug.debug("Waiting for applet to initialize...");
>>> - } catch (InterruptedException ie) {
>>> - // just wait
>>> - }
>>> - }
>>> + waitForAppletInit((NetxPanel) panel);
>>>
>>> PluginDebug.debug(panel + " -- " + panel.getApplet() + " -- " + ((NetxPanel) panel).isAlive());
>>>
>>> @@ -1548,10 +1682,11 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> if (countApplets() == 0) {
>>> appletSystemExit();
>>> }
>>> +
>>> + updateStatus(identifier, PAV_INIT_STATUS.DESTROYED);
>>> }
>>> }).start();
>>>
>>> - status.put(identifier, PAV_INIT_STATUS.INACTIVE);
>>> }
>>>
>>> /**
>>> @@ -1677,18 +1812,6 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> val = buf.toString();
>>> }
>>>
>>> - att = att.replace(">", ">");
>>> - att = att.replace("<", "<");
>>> - att = att.replace("&", "&");
>>> - att = att.replace(" ", "\n");
>>> - att = att.replace(" ", "\r");
>>> -
>>> - val = val.replace(">", ">");
>>> - val = val.replace("<", "<");
>>> - val = val.replace("&", "&");
>>> - val = val.replace(" ", "\n");
>>> - val = val.replace(" ", "\r");
>>> -
>>> PluginDebug.debug("PUT " + att + " = '" + val + "'");
>>> atts.put(att.toLowerCase(java.util.Locale.ENGLISH), val);
>>>
>>> @@ -1708,7 +1831,6 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> // private static final == inline
>>> private static final boolean isInt(Object o) {
>>> boolean isInt = false;
>>> -
>>> try {
>>> Integer.parseInt((String) o);
>>> isInt = true;
>>> @@ -1763,7 +1885,6 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> } catch (IOException ioe) {
>>> return ioe;
>>> }
>>> -
>>> return null;
>>> }
>>> };
>>> @@ -1785,6 +1906,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
>>> boolean isObjectTag = false;
>>> boolean isEmbedTag = false;
>>
More information about the distro-pkg-dev
mailing list