Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Don't use DesktopRootWindowHost init factory #468

Merged
merged 4 commits into from
Aug 29, 2013
Merged

Don't use DesktopRootWindowHost init factory #468

merged 4 commits into from
Aug 29, 2013

Conversation

cmarcelo
Copy link
Contributor

We were using a views::ViewsDelegate created for testing, it didn't provide much. By using our own implementation, we can hook DesktopRootWindowHostXWalk without needing to patch Chromium.

This series ends up cleaning some unused / redundant code as well.

@Trybot
Copy link

Trybot commented Aug 27, 2013

Trybot Dashboard


The building Status of PATCH

Thu Aug 29 2013 17:13:21 GMT+0800 (CST) [xwalk_linux] Passed LOGS: build successful
Thu Aug 29 2013 17:49:33 GMT+0800 (CST) [xwalk_win] Passed LOGS: build successful
Thu Aug 29 2013 16:48:09 GMT+0800 (CST) [xwalk_aura] Building...
Thu Aug 29 2013 17:49:34 GMT+0800 (CST) [xwalk_android] Passed LOGS: build successful

@cmarcelo
Copy link
Contributor Author

@luxtella @darktears

This would make the changes in Chromium Crosswalk unnecessary. What do you think?

@cmarcelo
Copy link
Contributor Author

I refer to one of the patches in crosswalk-project/chromium-crosswalk#42

There are other patches there, I'm trying whether subclassing DesktopRootWindowHostX11 ends up being a better choice (we may have to change it slightly to allow that).

@tiagovignatti
Copy link
Contributor

very nice! we should remove InitDesktopRootWindowHostFactory in Chromium's side then.

@ds-hwang
Copy link
Contributor

excellent! I could not imagine that it is possible. Perfectly satisfy our requirement.
LGTM!

This patch brings the special case for OnBeforeWidgetInit() function to
our own class which is the only thing we used from
DesktopTestViewsDelegate. This is a first step to inherit directly from
ViewsDelegate instead of relying in testing infrastructure.

Also removed g_views_delegate since views::ViewsDelegate already provide
a place to store it's application-wide instance.
Add just the necessary stubs like the TestViewsDelegate had. We still
don't make use of those hooks.

One difference is that TestViewsDelegate has the
SetUseTransparentWindows, but we never called it, so it's OK to keep the
virtual UseTransparentWindows() always false until we have some use for
it.
When using DesktopNativeWidgetAura, we can provide our own
DesktopRootWindowHost via the Widget::InitParams struct. The function
OnBeforeWidgetInit() provides a way to identify and create the hook at
the right time.

This removes the usage of the extra patch to add a Factory to
DesktopRootWindowHost.
@kenchris
Copy link
Contributor

lgtm

@cmarcelo
Copy link
Contributor Author

@luxtella I'm using the XWalk version on the desktop, so I won't include the ifdef change. Since when GTK is enable Aura isn't, this won't cause problems (things should keep working as they are today).

I frequently build the Aura version in Linux Desktop. I hope at some point we converge Linux to use only one variant.

@cmarcelo
Copy link
Contributor Author

Thanks for the review folks, I'll wait our trybots do their job before landing.

@cmarcelo
Copy link
Contributor Author

Trybot seems to be using old version of the patch. I fixed the style issue :/

darktears added a commit that referenced this pull request Aug 29, 2013
Don't use DesktopRootWindowHost init factory
@darktears darktears merged commit f4c71aa into crosswalk-project:master Aug 29, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants