Hi Caio! Thanks for your response and questions.
Q: - Do you see value in the application/ and runtime/ separation? Or we would be better serve by thinking they are one module? A: I do not have a strict opinion on this yet, I would postpone this question a bit and do the class re-design first. Q: - How much of this affects the Android port of Crosswalk. Do the application management features matter for them or only executing a standalone application? A: The affected modules are generic. The proposed class reorganization per se should not prevent from XWalk working on any platform. However, I believe we always can limit the maximum amount of running applications to 1 for a specific platform if needed, keeping the same design (guess XWalkCmdLineStrategy might be also useful here :) ). Q: What is Chromium adaptation? A: Probably I chose a wrong term. Here I meant a set of classes introduced just in order to implement Chromium content APIs - clients, delegates and so on. Classes that are required from any chromium-based browser and not specific to runtime functionality. Q: I think by "Not a singleton anymore" the goal is more to: don't make the rest of code depend on this too much. So it won't be a hanging area for main objects, is that correct? A: Right. XWalkContentBrowserClient is a kind of implementation detail for us. We'd better to use ApplicationManager(System) as a runtime "main" object. Q: I think this is pretty much what ApplicationSystem class is all about. The difference would be merging ApplicationService into it, which may not even be a good idea. ApplicationSystem does a lot of setup, it's nice to have a smaller/focused interface (ApplicationService) to interact with the actual functionality. A: OK, let's have ApplicationSystem and smaller interfaces. Q: Do we plan to have more than those two strategies. Compared to the other changes, this seems less important and if we don't use the flexibility, might be too much engineering. A: As for XWalkApplicationPageDisplayStrategy I strongly believe we should decouple an application page and the way it should be displayed. Those are different functionalities and at some point we might need to provide different UX on different platforms. BR, Mikhail ________________________________________ From: Oliveira, Caio Sent: Friday, November 22, 2013 8:11 PM To: Pozdnyakov, Mikhail Cc: [email protected] Subject: Re: [Crosswalk-dev] Proposal for design improvements in XWalk runtime Hi! This is very good! Some questions: - Do you see value in the application/ and runtime/ separation? Or we would be better serve by thinking they are one module? It seems you are steering in that direction already by "replacing" the Runtime with XWalkApplicationPage. It may be the case that we are drawing the line at the wrong level, and move more things to inside application would be enough. - How much of this affects the Android port of Crosswalk. Do the application management features matter for them or only executing a standalone application? On Fri, Nov 22, 2013 at 02:41:18PM +0000, Pozdnyakov, Mikhail wrote: > Main intentions: > - provide functionality that would allow to run several applications at the > same time > - decouple chromium adaptation and xwalk runtime logic > - make it easier to comprehend the code What is Chromium adaptation? > XWalkContentBrowserClient: > - Not a singleton anymore > - Will accept XWalkCmdLineStrategy object - a delegate(strategy) object for > handling the cmd line arguments. I think by "Not a singleton anymore" the goal is more to: don't make the rest of code depend on this too much. So it won't be a hanging area for main objects, is that correct? > XWalkCmdLineStrategy (maybe there is a better name): > - A strategy class encapsulating cmd line parsing and showing a browser > window or invoking an app (depending on cmd line parameters) > - Owned by XWalkContentBrowserClient > - XWalkCmdLineStrategy will encapsulate the logic currently present at > ApplicationSystem's HandleApplicationManagementCommands() and > LaunchFromCommandLine() methods and also at > part of XWalkBrowserMainParts::PreMainMessageLoopRun() logic. Good, but I consider this a minor improvement. > XWalkBrowserContext: > - The present RuntimeContext should be renamed to XWalkBrowserContext > - XWalkBrowserContext should not own any appliction classes > - XWalkBrowserContext is created by main parts and passed to an > XWalkApplicationManager object > XWalkApplicationManager: > - a new singleton class ( its instance is created by mainparts but there is > global function to access the instance from outside) > - Application manager is responsible for launching applications, managing the > running applications, installation/uninstallation of applications, providing > application services > - XWalkApplicationManager will substitute the present ApplicationSystem and > ApplicationService classes > - XWalkApplicationManager will own ApplicationStorage and > ApplicationEventManager I think this is pretty much what ApplicationSystem class is all about. The difference would be merging ApplicationService into it, which may not even be a good idea. ApplicationSystem does a lot of setup, it's nice to have a smaller/focused interface (ApplicationService) to interact with the actual functionality. I'd go even further and say that those changes are not crucial, and will be more about moving code around than actually functionality. > XWalkApplicationMetaData: > - The present Application class will be renamed to ApplicationMetaData. > > > XWalkApplication: > - A new class representing an application > - XWalkApplication class will encapsulate functionalty of present > ApplicationProcessManager, RuntimeRegistry and (partially) Runtime classes > - XWalkApplication will be owned be ApplicationManager > - XWalkApplication will contain ApplicationMetaData data member > - XWalkApplication will contain list of XWalkApplicationPages. This is the big missing piece in Crosswalk design, and IMHO the most important contribution from the proposal. I really think effort should be dedicated here and in the class below. > XWalkApplicationPage: > - A new class representing a web page (There's one-to-one correspondence > between WebContents and > XWalkApplicationPage). > - XWalkApplicationPage will encapsulate most of the logic currently present > in Runtime class > - XWalkApplicationPage will be owned by the corresponding XWalkApplication > instance. > XWalkApplicationPageDisplayStrategy (better name here is also possible :) ) > - A new strategy class encapsulating the logic for showing an application page > - This class will substitute present Runtime::AttachDefaultWindow and > Runtime::AttachWindow methods. > - No restricted ownership. In principal can be allocated even on stack. Do we plan to have more than those two strategies. Compared to the other changes, this seems less important and if we don't use the flexibility, might be too much engineering. > The design doc (and class diagram) is coming. Thanks for sharing, Cheers, Caio --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Crosswalk-dev mailing list [email protected] https://lists.crosswalk-project.org/mailman/listinfo/crosswalk-dev
