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
_______________________________________________
Crosswalk-dev mailing list
Crosswalk-dev@lists.crosswalk-project.org
https://lists.crosswalk-project.org/mailman/listinfo/crosswalk-dev

Reply via email to