[RFC][plugin]: fix concurrency problems in PAV.java

[Deepak Bhole]

* Denis Lila <dlila at redhat.com> [2011-04-14 11:43]:
> > It is not irrelevant to strongly type a container.
> 
> What I meant by irrelevant is that NetxPanel is a subclass, so it
> adds to the implementation of AppletPanel. We don't need any
> of what it adds, because the only reason we have appletPanels
> is so we can implement getApplet(String) and getApplets().
> These methods only need a container whose elements have an
> applet field, and Vector<AppletPanel> is enough for that.
> 
> > Strong typing will catch errors at compile time whereas weak
> > typing will show them at runtime.
> 
> The reason for that is because when a container is weakly typed
> we may put bad data in it and get ClassCastExceptions when we
> manually cast. In this case, however, we never do any casts
> because the type is exactly as strong as it needs to be,
> because all we need is the ".applet" member, and AppletPanel
> has it.
> 

I know we are only using at as AP right now. However that may change in
the future. And rather than relying on someone making the right change in
the future (as opposed to just casting it to NP), it is best to do the
right thing as we spot it a potential problem.

> Do you still think I should change it to NetxPanel?
> 

Yes please.

Cheers,
Deepak

> Regards,
> Denis.
> 
> ----- Original Message -----
> > * Denis Lila <dlila at redhat.com> [2011-04-14 10:07]:
> > > > NetxPanel inherits from AVP, so why is the cast needed? Does
> > > > NetxPanel
> > > > override the method?
> > >
> > > Yes, NetxPanel overrides the 3 methods that we need the casts for.
> > > Since they're protected, but NetxPanel is in a different package,
> > > the compiler gives an error.
> > >
> > 
> > Ah okay then, thats what I suspected too.
> > 
> > > > > That said, I wouldn't mind changing the type of appletPanels
> > > > > to Vector<AVP>. Do you still think I should?
> > > > >
> > > >
> > > > Actually, perhaps we should go one step up and just change it to
> > > > NetxPanel? We only store those in the vector anyway.
> > >
> > > We could, but I would rather not because we only use that vector
> > > for the "applet" field of the AppletPanel class. We don't need
> > > a NetxPanel for that. Using a NetxPanel would add irrelevant
> > > details.
> > >
> > 
> > 
> > Cheers,
> > Deepak
> > 
> > > Regards,
> > > Denis.
> > >
> > > ----- Original Message -----
> > > > * Denis Lila <dlila at redhat.com> [2011-04-14 09:45]:
> > > > > > Instead of doing this, it would be easier to change the type
> > > > > > of
> > > > > > appletPanels to Vector<AppletViewerPanel> , then the casting
> > > > > > is
> > > > > > not
> > > > > > needed as AVP inherits from AP.
> > > > >
> > > > > The casting to AppletViewerPanel is not caused by appletPanels.
> > > > > It
> > > > > is caused by the "panel" member. It used to be declared as an
> > > > > AVP,
> > > > > then I changed it to a NetxPanel, which made the 3 new casts
> > > > > necessary (but it eliminated many more casts than it added).
> > > > >
> > > >
> > >
> > > >
> > >
> > > >
> > > > Cheers,
> > > > Deepak
> > > >
> > > > > Thank you,
> > > > > Denis.
> > > > >
> > > > > ----- Original Message -----
> > > > > > * Denis Lila <dlila at redhat.com> [2011-04-13 15:10]:
> > > > > > > > If a patch akin to
> > > > > > > > http://hg.openjdk.java.net/jdk6/jdk6/langtools/rev/5c2858bccb3f
> > > > > > > > goes in, I will not be responsible for my actions.
> > > > > > >
> > > > > > > I definitely don't want that ;-).
> > > > > > >
> > > > > > > Attached is the second patch. It's mostly cleanup, and it
> > > > > > > also uses "pav" in destroyApplet, instead of calling
> > > > > > > get(id) again and again.
> > > > > > >
> > > > > > > Ok to push?
> > > > > > >
> > > > > >
> > > > > > ...
> > > > > > ...
> > > > > >
> > > > > > <snip>
> > > > > > >
> > > > > > >              PluginDebug.debug("getCachedImageRef() getting
> > > > > > >              img
> > > > > > >              from
> > > > > > >              URL = ", url);
> > > > > > > @@ -910,8 +815,8 @@
> > > > > > >          SocketPermission panelSp =
> > > > > > >                  new
> > > > > > >                  SocketPermission(panel.getCodeBase().getHost(),
> > > > > > >                  "connect");
> > > > > > >          synchronized(appletPanels) {
> > > > > > > - for (Enumeration e = appletPanels.elements();
> > > > > > > e.hasMoreElements();) {
> > > > > > > - AppletPanel p = (AppletPanel) e.nextElement();
> > > > > > > + for (Enumeration<AppletPanel> e = appletPanels.elements();
> > > > > > > e.hasMoreElements();) {
> > > > > > > + AppletPanel p = e.nextElement();
> > > > > > >                  String param = p.getParameter("name");
> > > > > > >                  if (param != null) {
> > > > > > >                      param = param.toLowerCase();
> > > > > > > @@ -1591,15 +1496,15 @@
> > > > > > >           * at the same time.
> > > > > > >           */
> > > > > > >          try {
> > > > > > > - panel.joinAppletThread();
> > > > > > > - panel.release();
> > > > > > > + ((AppletViewerPanel)panel).joinAppletThread();
> > > > > > > + ((AppletViewerPanel)panel).release();
> > > > > > >          } catch (InterruptedException e) {
> > > > > > >              return; // abort the reload
> > > > > > >          }
> > > > > > >
> > > > > > >          AccessController.doPrivileged(new
> > > > > > >          PrivilegedAction<Void>()
> > > > > > >          {
> > > > > > >              public Void run() {
> > > > > > > - panel.createAppletThread();
> > > > > > > + ((AppletViewerPanel)panel).createAppletThread();
> > > > > > >                  return null;
> > > > > > >              }
> > > > > >
> > > > > >
> > > > >