Downloading Referrers Grabs Focus

[ingalls]

When Downloading nodes through the remote, JOSM grabs focus each time a 'downloading referrs' dialogue appears. Since there is a new dialogue for each and every node being downloaded, large remote downloads make the host computer unusable as JOSM is constantly grabbing focus.

[Don-vip]

Request example:

http://localhost:8111/load_object?objects=n357622684,n357623764,n2493782437,n2493782445,n2493782455,n2493782475,n2493782517,n2493782529,n2493782532,n2493782536,n2493782559,n2493782562,n2493782610,n2493782616,n2493782625,n2493782632,n2493782646,n2493782661,n2493863533,n2493863668,n2493964455,n2493964594,n2493964947,n2493964952,n2493983896,n2493983907,n2493983922,n2493983970,n2493983979,n2493983984,n2493984002,n2493984008,n2493984036,n2493984054,n2493984212,n2493984268,n2493984282,n2494002191,n2494002218,n2494002229,n2494002236,n2494002239,n2494002249,n2494002251,n2494002266,n2494002308,n2494002313,n2494002317,n2494002331,n2494002337,n2494002376,n2494002384,n2494002654,n2494002769,n2494002794,n2494002851,n2494018287,n2494018288,n2494018292,n2494018299,n2494018302,n2494018304,n2494018314,n2494018331,n2494018368,n2494018387,n2494018390,n2494018401,n2494018410,n2494018419,n2494023442,n2494023445,n2494023704,n2494023817,n2494023866,n2494027649,n2494027652,n2494027654,n2494027672,n2494027675,n2494033294,n2494041724,n2494045819,n2494045824,n2494045850,n2494045899,n2494045951,n2494046073,n2494046075,n2494046116,n2494046272,n2494046283,n2494046290,n2494046324,n2494046330,n2494046333,n2494046335,n2494046458

[Balaitous]

I am trying to do something. But I am not friendly with thread, and I don't understand every thing.
One of the problem is how to do Cancel.
It's rather easy to do a Stop button (stop downloading next referrers), but a real Cancel button need to remove all downloading object !

I put code here since in diff file it's not easy to see (it's mainly the older code, but reoder)
I have put some FIXME comment where I have problems.

    /**
     * @param newLayer if the data should be downloaded into a new layer
     * @param ids
     * @param downloadReferrers if the referrers of the object should be downloaded as well, i.e., parent relations, and for nodes, additionally, parent ways
     * @param full if the members of a relation should be downloaded as well
     */
    public static void processItems(boolean newLayer, final List<PrimitiveId> ids, boolean downloadReferrers, boolean full) {
 
        class DownloadPrimitivesWithReferrersTask extends PleaseWaitRunnable {
            final OsmDataLayer layer;
            List<PrimitiveId> ids;
            boolean full;
            DownloadPrimitivesTask task;
            
            DownloadPrimitivesWithReferrersTask(final OsmDataLayer layer, List<PrimitiveId> ids,
                    boolean full) { //, ProgressMonitor monitor) {
                super(tr("Download objects"), false);
                this.layer = layer;
                this.ids = ids;
                this.full = full;
            }
            
            @Override
            protected void cancel() {
                // FIXME: how to do ?
            }

            @Override
            protected void realRun() throws SAXException, IOException, OsmTransferException {
                task = new DownloadPrimitivesTask(layer, ids, full, getProgressMonitor().createSubTaskMonitor(1, false));
                // FIXME: Need to run task in an other thread (like Main.worker.submit(task) ?
                task.run();
                getProgressMonitor().setTicksCount(ids.size());
                for(PrimitiveId id : ids) {
                    System.out.printf("referrers task %s\n", id.toString());
                    // FIXME: In an other thread ?
                    new DownloadReferrersTask(layer, id, getProgressMonitor().createSubTaskMonitor(1, false)).run();
                }
            }

            @Override
            protected void finish() {
                // FIXME: Does all task are really finish ?
                System.out.printf("call to finish\n");
                Runnable showErrorsAndWarnings = new Runnable() {
                    @Override
                    public void run() {
                        final Set<PrimitiveId> errs = task.getMissingPrimitives();
                        if (errs != null && !errs.isEmpty()) {
                            try {
                                SwingUtilities.invokeAndWait(new Runnable() {
                                    @Override
                                    public void run() {
                                        reportProblemDialog(errs,
                                                trn("Object could not be downloaded", "Some objects could not be downloaded", errs.size()),
                                                trn("One object could not be downloaded.<br>",
                                                    "{0} objects could not be downloaded.<br>",
                                                    errs.size(),
                                                    errs.size())
                                                + tr("The server replied with response code 404.<br>"
                                                    + "This usually means, the server does not know an object with the requested id."),
                                                tr("missing objects:"),
                                                JOptionPane.ERROR_MESSAGE
                                        ).showDialog();
                                    }
                                });
                            } catch (InterruptedException ex) {
                                Main.warn("InterruptedException while displaying error dialog");
                            } catch (InvocationTargetException ex) {
                                Main.warn(ex);
                            }
                        }

                        final Set<PrimitiveId> del = new TreeSet<PrimitiveId>();
                        DataSet ds = getCurrentDataSet();
                        for (PrimitiveId id : ids) {
                            OsmPrimitive osm = ds.getPrimitiveById(id);
                            if (osm != null && osm.isDeleted()) {
                                del.add(id);
                            }
                        }
                        if (!del.isEmpty()) {
                            SwingUtilities.invokeLater(new Runnable() {
                                @Override
                                public void run() {
                                    reportProblemDialog(del,
                                            trn("Object deleted", "Objects deleted", del.size()),
                                            trn(
                                                "One downloaded object is deleted.",
                                                "{0} downloaded objects are deleted.",
                                                del.size(),
                                                del.size()),
                                            null,
                                            JOptionPane.WARNING_MESSAGE
                                    ).showDialog();
                                }
                            });
                        }
                    }
                };
                Main.worker.submit(showErrorsAndWarnings);
            }
        }
        
        System.out.printf("DEBUG: DownloadPrimitiveAction.processItems\n");
        OsmDataLayer layer = getEditLayer();
        if ((layer == null) || newLayer) {
            layer = new OsmDataLayer(new DataSet(), OsmDataLayer.createNewName(), null);
            Main.main.addLayer(layer);
        }

        final DownloadPrimitivesWithReferrersTask task = 
                new DownloadPrimitivesWithReferrersTask(layer, ids, full); //, monitor);
        Main.worker.submit(task);

        System.out.printf("task done\n");
    }

[Balaitous]

Draft of current work

[bastiK]

Replying to Balaitous:

I am trying to do something. But I am not friendly with thread, and I don't understand every thing.
One of the problem is how to do Cancel.
It's rather easy to do a Stop button (stop downloading next referrers), but a real Cancel button need to remove all downloading object !

I put code here since in diff file it's not easy to see (it's mainly the older code, but reoder)
I have put some FIXME comment where I have problems.

Hi Balaitous,

Looks good so far! For cancel, you could save the DownloadPrimitivesTask and the DownloadReferrersTasks in fields, so they can be accessed from the cancel method. Then do something like

            @Override
            protected void cancel() {
                if (task != null) {
                    task.cancel();
                }
                // ...
            }

I think it is correct to run the tasks directly as you do, without any additional parallel processing. This ensures that they are indeed finished, when finish() is invoked.

Make sure you don't forget the check for the downloadReferrers flag. Also you may want to check if the handling of the ProgressMonitor can be improved. You may have to play a little bit with the options and look at examples, the code for progress update is not too well documented.

[Balaitous]

Draft of current work

[Balaitous]

I have try to make download into a temporary layer, but a lot of problems remains.
Maybe it should be better to adapt DownloadPrimitivesTask class be setting a downloadReferrers flag, and work with dataset instead of layer.

[bastiK]

Replying to Balaitous:

I have try to make download into a temporary layer, but a lot of problems remains.

What kind of problems?

[Balaitous]

In order to be able to do a full cancel, all primitives and referrers have to be downloaded into a tmpLayer that is not attach to any mapFrame.
This involved that it is possible to call DownloadPrimitivesTask and DownloadReferrersTask before Main.map init.
So I have had test if(Main.map != null).
Another problem is that when action is cancelled, primitives from the tmpLayer remains selected.

[Balaitous]

Draft of current work, I have found where downloaded primitives are selected. In a good way.

[Balaitous]

I think this time it is OK

[bastiK]

In 6973/josm:

applied #9634 - Downloading Referrers Grabs Focus (patch by Balaitous, minor modifications)

[bastiK]

Great work! I did minor changes to avoid EDT violation.

[Balaitous]

I don't understand every thing with thread. I have a little experience with MPI and openMP, but nothing with java!

[bastiK]

Then you probably know more about threads than me. Just take care that GUI stuff (like changing the selection) is done in event dispatch thread (EDT). (And everything that takes a long time, like download, must not be done in EDT, otherwise the program will freeze.) I only noticed because there were some warnings on the command line.

[Don-vip]

EDT violations are easy to spot. Just look at warnings printed on Console when you run the program from a local build (i.e Eclipse) :)

[Don-vip]

In 6980/josm:

see #9634 - fix sonar issue - inconsistent synchronization