search

intel / ozone-wayland

Wayland implementation for Chromium Ozone classes
BSD 3-Clause "New" or "Revised" License
219 stars 82 forks source link

Avoid un-necessary dependencies for different targets. #204

Closed kalyankondapally closed 9 years ago

kalyankondapally commented 10 years ago

As things have settled down stability wise, we should continue doing the work from previous release of avoiding unnecessary layer violations.

Even though the target is to achieve it before next Milestone, we need to avoid any regressions.

Dependency: None. Blocks: None. SubTasks: #205, #208. #215

kalyankondapally commented 10 years ago

https://github.com/kalyankondapally/ozone-wayland/commit/645903efe907b2503ec26224ffc44115faee1cac https://github.com/kalyankondapally/ozone-wayland/commit/f34b308707a34f7cb9110af6d7a62773d93658db https://github.com/kalyankondapally/ozone-wayland/commit/2f86f496dac75cfac02cf963da3fdf9d6632d1cf

First set.

@tiagovignatti PTAL

tiagovignatti commented 10 years ago

I was actually starting to design something to fix this in my head, therefore I'm very glad you brought this in time here @kalyankondapally. Thank you.

I think those three patches are definitely going to the right direction, but for me there are a couple of things we need to understand better before starting to actually cook patches. I see that the following are the most important ones to be addressed:

  1. untie Ozone-Wayland from Views, for targets that don't depend on it
  2. turn class OzoneDisplay independent from Content API

take for example 'gpu_unittests' target (I think 'aura_demo' has similar problems). Although it shouldn't rely on views layer (it's a higher level concept), it's currently trying to build views::DesktopFactoryOzone implementation which is very wrong. I was thinking that maybe the OzonePlatformWayland has to call DesktopFactoryOzone, which in turn dynamically load the dependencies (is it feasible? dunno).

Besides, we're referencing content API (content::ChildProcess::current(), among others) inside Ozone-Wayland and gpu_unittests don't actually depend on it, which fails the compilation with undefined symbols; most of the dependencies are related to the OzoneDisplayChannel actually and I'm not sure how to address this.

wdyt?

kalyankondapally commented 10 years ago

we have few things to fix and you have already identified some of them. 1)Do not include any views dependencies when not needed. (This is more of a target issue rather than unit tests). 2)Make our code independent of content. 3)Don't bundle views (when building with toolkit_views) as part of Ozone. I have couple of ideas for this which I need to play around and see.

I see these are initial steps and once done, we can take things from their on.

kalyankondapally commented 10 years ago

The importance of Step 1

We do have an "Embedded" target which is a minimum content shell. This disables Toolkit Views.

This should get us working with a minimum set of tests and see if we need to fix them for Wayland while trying to separate Desktop dependencies from Ozone target.

kalyankondapally commented 9 years ago

Solved everything in scope of this bug. We should open new issues as found and fix them.