4714674: JEditorPane.setPage(url) blocks AWT thread when HTTP protocol is used

Summary: Both POST and GET can now be processed asynchronously; PageLoader refactored
Reviewed-by: gsm

src/share/classes/javax/swing/JEditorPane.java

@@ -429,9 +429,8 @@ public class JEditorPane extends JTextComponent {
             // different url or POST method, load the new content
 
             int p = getAsynchronousLoadPriority(getDocument());
-            if ((postData == null) || (p < 0)) {
-                // Either we do not have POST data, or should submit the data
-                // synchronously.
+            if (p < 0) {
+                // open stream synchronously
                 InputStream in = getStream(page);
                 if (kit != null) {
                     Document doc = initializeModel(kit, page);
@@ -440,22 +439,13 @@ public class JEditorPane extends JTextComponent {
                     // view notifications slowing it down (i.e. best synchronous
                     // behavior) or set the model and start to feed it on a separate
                     // thread (best asynchronous behavior).
-                    synchronized(this) {
-                        if (loading != null) {
-                            // we are loading asynchronously, so we need to cancel
-                            // the old stream.
-                            loading.cancel();
-                            loading = null;
-                        }
-                    }
                     p = getAsynchronousLoadPriority(doc);
                     if (p >= 0) {
                         // load asynchronously
                         setDocument(doc);
                         synchronized(this) {
-                            loading = new PageStream(in);
-                            Thread pl = new PageLoader(doc, loading, p, loaded, page);
-                            pl.start();
+                            pageLoader = new PageLoader(doc, in, loaded, page);
+                            pageLoader.execute();
                         }
                         return;
                     }
@@ -464,11 +454,15 @@ public class JEditorPane extends JTextComponent {
                     reloaded = true;
                 }
             } else {
-                // We have POST data and should send it asynchronously.
-                // Send (and subsequentally read) data in separate thread.
+                // we may need to cancel background loading
+                if (pageLoader != null) {
+                    pageLoader.cancel(true);
+                }
+
+                // Do everything in a background thread.
                 // Model initialization is deferred to that thread, too.
-                Thread pl = new PageLoader(null, null, p, loaded, page);
-                pl.start();
+                pageLoader = new PageLoader(null, null, loaded, page);
+                pageLoader.execute();
                 return;
             }
         }
@@ -604,44 +598,38 @@ public class JEditorPane extends JTextComponent {
 
 
     /**
-     * Thread to load a stream into the text document model.
+     * Loads a stream into the text document model.
      */
-    class PageLoader extends Thread {
+    class PageLoader extends SwingWorker<URL, Object> {
 
         /**
          * Construct an asynchronous page loader.
          */
-        PageLoader(Document doc, InputStream in, int priority, URL old,
-                   URL page) {
-            setPriority(priority);
+        PageLoader(Document doc, InputStream in, URL old, URL page) {
             this.in = in;
             this.old = old;
             this.page = page;
             this.doc = doc;
         }
 
-        boolean pageLoaded = false;
-
         /**
          * Try to load the document, then scroll the view
          * to the reference (if specified).  When done, fire
          * a page property change event.
          */
-        public void run() {
+        protected URL doInBackground() {
+            boolean pageLoaded = false;
             try {
                 if (in == null) {
                     in = getStream(page);
                     if (kit == null) {
                         // We received document of unknown content type.
-                        UIManager.getLookAndFeel().provideErrorFeedback(
-                                JEditorPane.this);
-                        return;
-                    }
-                    // Access to <code>loading</code> should be synchronized.
-                    synchronized(JEditorPane.this) {
-                        in = loading = new PageStream(in);
+                        UIManager.getLookAndFeel().
+                                provideErrorFeedback(JEditorPane.this);
+                        return old;
                     }
                 }
+
                 if (doc == null) {
                     try {
                         SwingUtilities.invokeAndWait(new Runnable() {
@@ -653,11 +641,11 @@ public class JEditorPane extends JTextComponent {
                     } catch (InvocationTargetException ex) {
                         UIManager.getLookAndFeel().provideErrorFeedback(
                                                             JEditorPane.this);
-                        return;
+                        return old;
                     } catch (InterruptedException ex) {
                         UIManager.getLookAndFeel().provideErrorFeedback(
                                                             JEditorPane.this);
-                        return;
+                        return old;
                     }
                 }
 
@@ -682,16 +670,14 @@ public class JEditorPane extends JTextComponent {
             } catch (IOException ioe) {
                 UIManager.getLookAndFeel().provideErrorFeedback(JEditorPane.this);
             } finally {
-                synchronized(JEditorPane.this) {
-                    loading = null;
-                }
-                SwingUtilities.invokeLater(new Runnable() {
-                    public void run() {
-                        if (pageLoaded) {
-                            firePropertyChange("page", old, page);
+                if (pageLoaded) {
+                    SwingUtilities.invokeLater(new Runnable() {
+                        public void run() {
+                            JEditorPane.this.firePropertyChange("page", old, page);
                         }
-                    }
-                });
+                    });
+                }
+                return (pageLoaded ? page : old);
             }
         }
 
@@ -718,51 +704,6 @@ public class JEditorPane extends JTextComponent {
         Document doc;
     }
 
-    static class PageStream extends FilterInputStream {
-
-        boolean canceled;
-
-        public PageStream(InputStream i) {
-            super(i);
-            canceled = false;
-        }
-
-        /**
-         * Cancel the loading of the stream by throwing
-         * an IOException on the next request.
-         */
-        public synchronized void cancel() {
-            canceled = true;
-        }
-
-        protected synchronized void checkCanceled() throws IOException {
-            if (canceled) {
-                throw new IOException("page canceled");
-            }
-        }
-
-        public int read() throws IOException {
-            checkCanceled();
-            return super.read();
-        }
-
-        public long skip(long n) throws IOException {
-            checkCanceled();
-            return super.skip(n);
-        }
-
-        public int available() throws IOException {
-            checkCanceled();
-            return super.available();
-        }
-
-        public void reset() throws IOException {
-            checkCanceled();
-            super.reset();
-        }
-
-    }
-
     /**
      * Fetches a stream for the given URL, which is about to
      * be loaded by the <code>setPage</code> method.  By
@@ -1573,11 +1514,7 @@ public class JEditorPane extends JTextComponent {
 
     // --- variables ---------------------------------------
 
-    /**
-     * Stream currently loading asynchronously (potentially cancelable).
-     * Access to this variable should be synchronized.
-     */
-    PageStream loading;
+    private SwingWorker<URL, Object> pageLoader;
 
     /**
      * Current content binding of the editor.

test/javax/swing/JEditorPane/bug4714674.java

+/* @test
+   @bug 4714674
+   @summary Tests that JEditorPane opens HTTP connection asynchronously
+   @author Peter Zhelezniakov
+   @run main bug4714674
+*/
+
+import javax.swing.*;
+
+import com.sun.net.httpserver.HttpExchange;
+import com.sun.net.httpserver.HttpHandler;
+import com.sun.net.httpserver.HttpServer;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.concurrent.Executors;
+
+
+public class bug4714674 {
+
+    public static void main(String[] args) throws Exception {
+        new bug4714674().test();
+    }
+
+    private void test() throws Exception {
+        final DeafServer server = new DeafServer();
+        final String baseURL = "http://localhost:" + server.getPort() + "/";
+
+        SwingUtilities.invokeLater(new Runnable() {
+            public void run() {
+                try {
+                    JEditorPane pane = new JEditorPane();
+                    ((javax.swing.text.AbstractDocument)pane.getDocument()).
+                            setAsynchronousLoadPriority(1);
+
+                    // this will block EDT unless 4714674 is fixed
+                    pane.setPage(baseURL);
+                } catch (IOException e) {
+                    // should not happen
+                    throw new Error(e);
+                }
+            }
+        });
+
+        // if 4714674 is fixed, this executes before connection times out
+        SwingUtilities.invokeLater(new Runnable() {
+            public void run() {
+                synchronized (server) {
+                    server.wakeup = true;
+                    server.notifyAll();
+                }
+                pass();
+            }
+        });
+
+        // wait, then check test status
+        try {
+            Thread.sleep(5000);
+            if (!passed()) {
+                throw new RuntimeException("Failed: EDT was blocked");
+            }
+        } finally {
+            server.destroy();
+        }
+    }
+
+    private boolean passed = false;
+
+    private synchronized boolean passed() {
+        return passed;
+    }
+
+    private synchronized void pass() {
+        passed = true;
+    }
+}
+
+/**
+ * A "deaf" HTTP server that does not respond to requests.
+ */
+class DeafServer {
+    HttpServer server;
+    boolean wakeup = false;
+
+    /**
+     * Create and start the HTTP server.
+     */
+    public DeafServer() throws IOException {
+        InetSocketAddress addr = new InetSocketAddress(0);
+        server = HttpServer.create(addr, 0);
+        HttpHandler handler = new DeafHandler();
+        server.createContext("/", handler);
+        server.setExecutor(Executors.newCachedThreadPool());
+        server.start();
+    }
+
+    /**
+     * Stop server without any delay.
+     */
+    public void destroy() {
+        server.stop(0);
+    }
+
+    /**
+     * Return actual server port number, useful for constructing request URIs.
+     */
+    public int getPort() {
+        return server.getAddress().getPort();
+    }
+
+
+    class DeafHandler implements HttpHandler {
+        public void handle(HttpExchange r) throws IOException {
+            synchronized (DeafServer.this) {
+                while (! wakeup) {
+                    try {
+                        DeafServer.this.wait();
+                    } catch (InterruptedException e) {
+                        // just wait again
+                    }
+                }
+            }
+        }
+    }
+}