I forget to send to all, when answering. Sorry. Tl'tr
While answering I racked my head about one problem: If a dialog have a fix browser and a browser have a fix page, for the whole lifetime - why not let the browser use the renderers directly without using the BrowserPage construct. As we want to get rid of the BrowserPage has Browserfunctions relationship [1], I believe we could get rid of the BrowserPage classes all together. This class only holding the path to the html file (which isn't more than a String in the end). I don't know if Christian still reading this mailinglist, but was there a problem in IntelliJ that actualy made the replacement of the browser in a shell necessary? I have vague remembrance, that minimizing the view forces a browser replacement? But I could be very wrong here... [1] http://saros-build.imp.fu-berlin.de/gerrit/#/c/3033/1/de.fu_berlin.inf.dpp.ui/src/de/fu_berlin/inf/dpp/ui/ide_embedding/BrowserCreator.java 2016-04-08 14:28 GMT+02:00 Matthias Bohnstedt <matthias.bohnst...@gmail.com> : > > > 2016-04-07 19:09 GMT+02:00 Zieris, Franz <franz.zie...@fu-berlin.de>: > >> Hi there, >> >> I started thinking about a few more test cases for the UI project >> and I came along the BrowserManager [1]. >> >> I don't fully understand the idea of the "setBrowser" method. >> It says that it "sets or replaces there browser for the given page >> and all corresponding renderers". >> Is there a reason for the "replace" part? >> > > I don't think that the browser is really "replaced" literal sense of the > word at any time. If a dialog shell or a view with a html page is shown, > the browser is never replaced during the lifetime of that shell. Even if > the BrowserManager says that "the browser instance for one window may > change, this class allows the replacement of each browser"[1], we don't do > this at all. And I don't see any reason to do so in the future. It's a > relict from a very early stage of the HTML UI. The only time a new browser > is created (and setBrowser is called), is when a new shell (aka a dialog or > a view) is created. > > >> I guess these paths will be called multiple times during a plugins >> lifetime. Every time "createBrowser" is called, a new Browser object >> is created and passed to the BrowserManager. >> >> > Yes this is the case. > > Now consider the hypothetical calls (e.g. in a test case): >> > mgr.setBrowser(mainPage, browserOne); >> > mgr.setBrowser(mainPage, browserTwo); >> >> > This is indeed just hypothetical, because the mainPage should never be > displayed in another view than the SarosViewBrowserVersion. While you > *could* open a Dialog with the mainPage, by calling > DialogManager.showDialogWindow(MainPage), which would lead to a change of > the mapping as you mentioned it, you shouldn't. And I don't see any use > case to do so. And therefore no such test case. > > Ever other dialog (with it's browser page and the corresponding browser > instance) is registered in the DialogManager. You shouldn't been able to > open the same BrowserPage multiple times, as the dialog manager won't call > BrowserCreator.createBrowser() -->.setBrowser() in this case.[2] The > MainPage is the only page that is not registered in the DialogManager. > > If you look at the source code of BrowserManager.setBrowser, you'll see >> that the old browserOne will indeed be replaced in the page-to-browser >> mapping by the new browserTwo. >> But what about the renderers? >> According to the source code, each of the two renderers that the MainPage >> uses (stateRenderer and accountRenderer) now has *both* browserOne and >> browserTwo in its "browserList". This is because "setBrowser" only >> calls "renderer.addBrowser()" but not "removeBrowser" -- which is called >> every time a Browser is disposed. >> >> This is because the only time a browser actualy needs to be removed is > when the shell (with the browser) is disposed. > > >> So, again my question: What about the "replace" part of the "setBrowser" >> method? It sounds like the old browser object (browserOne in the example >> above) is "relieved of its duty", but until it is disposed, it will >> still provided with the events generated by the renderers. >> > > Again: There is no actual replacement of a browser during the lifetime of > its containing shell. So the disposing and 'relieve of its duty' should > happen at the same time. > > >> Or in other words: What would be valid test cases? >> Is the example above simply a non-correct usage of the BrowserManager? >> > > I don't think so. This is not the way these method is used. (At least it > shouldn't be). > > The question I got while I looked thought this classes is: Why we have 2 > mappings that seems to manage kind of the same thing and what's there > correlation? > > BrowserManager: > Map<Class<? extends IBrowserPage>, IJQueryBrowser> Page:Browser > > DialogManager[3] > Map<String, IBrowserDialog> openDialogs String (=the path of the html > file) : Dialog > > Since the BrowserPage and the the BrowserPage.getRelativePath() are > semantically even, we got 2 mappings Page:Browser and Page:Dialog. Because > a browser dies and lives with it's dialog (wrapping shell), wouldn't it > make sense to have a single Browser:Dialog mapping instead. The browser > already contain the page it is displaying (handed over on it's creation), > and this page is never changed during the browser lifetime. > > [1] > https://github.com/saros-project/saros/blob/master/de.fu_berlin.inf.dpp.ui/src/de/fu_berlin/inf/dpp/ui/manager/BrowserManager.java#L12 > [2] > https://github.com/saros-project/saros/blob/master/de.fu_berlin.inf.dpp.ui/src/de/fu_berlin/inf/dpp/ui/ide_embedding/DialogManager.java#L49 > [3] > https://github.com/saros-project/saros/blob/master/de.fu_berlin.inf.dpp.ui/src/de/fu_berlin/inf/dpp/ui/ide_embedding/DialogManager.java#L24 > > Cheers, > Matthias >
------------------------------------------------------------------------------
_______________________________________________ DPP-Devel mailing list DPP-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dpp-devel
