Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
9317111:
Fix views compilation on Linux with toolkit_views=1. (Closed)
|
Created:
8 years, 10 months ago by tfarina Modified:
8 years, 10 months ago CC:
chromium-reviews, msw+watch_chromium.org, alicet1 Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionFix views compilation on Linux with toolkit_views=1.
This fixes the following error:
../../ui/views/bubble/bubble_delegate.cc: In function ‘views::Widget* views::<unnamed>::CreateBubbleWidget(views::BubbleDelegateView*, views::Widget*)’:
../../ui/views/bubble/bubble_delegate.cc:29:50: error: cannot convert ‘GtkWindow*’ to ‘GtkWidget*’ in assignment
BUG=113010
TEST=none
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=120794
Patch Set 1 #
Messages
Total messages: 10 (0 generated)
alice: FYI. This is necessary because Widget::InitParams::parent is declared as gfx::NativeView not gfx::NativeWindow. And on GTK they are defined as GtkWidget/GtkWindow, but on Aura it doesn't make difference because it's defined as aura::Window.
Thiago, Views+gtk is going to be ripped out soon (it's being replaced by aura). Don't bother with this sort of stuff. -Scott On Sun, Feb 5, 2012 at 3:15 PM, <tfarina@chromium.org> wrote: > Reviewers: sky, > > Message: > alice: FYI. > > This is necessary because Widget::InitParams::parent is declared as > gfx::NativeView not gfx::NativeWindow. And on GTK they are defined as > GtkWidget/GtkWindow, but on Aura it doesn't make difference because it's > defined > as aura::Window. > > Description: > Fix views compilation on Linux with toolkit_views=1. > > This fixes the following error: > ../../ui/views/bubble/bubble_delegate.cc: In function ‘views::Widget* > views::<unnamed>::CreateBubbleWidget(views::BubbleDelegateView*, > views::Widget*)’: > ../../ui/views/bubble/bubble_delegate.cc:29:50: error: cannot convert > ‘GtkWindow*’ to ‘GtkWidget*’ in assignment > > R=sky@chromium.org > > > Please review this at http://codereview.chromium.org/9317111/ > > SVN Base: svn://svn.chromium.org/chrome/trunk/src > > Affected files: > M ui/views/bubble/bubble_delegate.h > > > Index: ui/views/bubble/bubble_delegate.h > diff --git a/ui/views/bubble/bubble_delegate.h > b/ui/views/bubble/bubble_delegate.h > index > 81fd9c84208953e86de88ff9de7e58f46fe5fc80..c7f55642650da135c857cf932c62f69219c8d820 > 100644 > --- a/ui/views/bubble/bubble_delegate.h > +++ b/ui/views/bubble/bubble_delegate.h > @@ -72,8 +72,8 @@ class VIEWS_EXPORT BubbleDelegateView : public > WidgetDelegateView, > int margin() const { return margin_; } > void set_margin(int margin) { margin_ = margin; } > > - gfx::NativeWindow parent_window() const { return parent_window_; } > - void set_parent_window(gfx::NativeWindow window) { parent_window_ = > window; } > + gfx::NativeView parent_window() const { return parent_window_; } > + void set_parent_window(gfx::NativeView window) { parent_window_ = window; > } > > bool use_focusless() const { return use_focusless_; } > void set_use_focusless(bool use_focusless) { > @@ -156,7 +156,7 @@ class VIEWS_EXPORT BubbleDelegateView : public > WidgetDelegateView, > bool use_focusless_; > > // Parent native window of the bubble. > - gfx::NativeWindow parent_window_; > + gfx::NativeView parent_window_; > > DISALLOW_COPY_AND_ASSIGN(BubbleDelegateView); > }; > >
On 2012/02/06 16:19:46, sky wrote: > Thiago, > > Views+gtk is going to be ripped out soon (it's being replaced by > aura). Don't bother with this sort of stuff. > I assume so toolkit_views is not going to be supported? Which are the flags that are supposed to work on Linux to compile and run views_examples_exe?
On Mon, Feb 6, 2012 at 8:35 AM, <tfarina@chromium.org> wrote: > On 2012/02/06 16:19:46, sky wrote: >> >> Thiago, > > >> Views+gtk is going to be ripped out soon (it's being replaced by >> aura). Don't bother with this sort of stuff. > > > I assume so toolkit_views is not going to be supported? That's right. Once we cut to aura we'll remove all the views+gtk code. > Which are the flags > that > are supposed to work on Linux to compile and run views_examples_exe? use_aura=1 . Most of us are primarily using use_aura=1 and chromeos=1. -Scott
On Mon, Feb 6, 2012 at 2:51 PM, Scott Violet <sky@chromium.org> wrote: > On Mon, Feb 6, 2012 at 8:35 AM, <tfarina@chromium.org> wrote: >> On 2012/02/06 16:19:46, sky wrote: >>> >>> Thiago, >> >> >>> Views+gtk is going to be ripped out soon (it's being replaced by >>> aura). Don't bother with this sort of stuff. >> >> >> I assume so toolkit_views is not going to be supported? > > That's right. Once we cut to aura we'll remove all the views+gtk code. > >> Which are the flags >> that >> are supposed to work on Linux to compile and run views_examples_exe? > > use_aura=1 . Most of us are primarily using use_aura=1 and chromeos=1. > views_examples_exe seems to be already broken on Linux with use_aura=1. ../../ui/views/examples/examples_main.cc: In function ‘int main(int, char**)’: ../../ui/views/examples/examples_main.cc:26:15: error: ‘g_type_init’ was not declared in this scope ../../ui/views/examples/examples_main.cc:27:24: error: ‘gtk_init’ was not declared in this scope > -Scott -- Thiago
Indeed. The way to run views_examples with aura is to build and run ash_shell, then click on the button that says views examples. Not ideal, but it works. -Scott On Mon, Feb 6, 2012 at 10:47 AM, Thiago Farina <tfarina@chromium.org> wrote: > On Mon, Feb 6, 2012 at 2:51 PM, Scott Violet <sky@chromium.org> wrote: >> On Mon, Feb 6, 2012 at 8:35 AM, <tfarina@chromium.org> wrote: >>> On 2012/02/06 16:19:46, sky wrote: >>>> >>>> Thiago, >>> >>> >>>> Views+gtk is going to be ripped out soon (it's being replaced by >>>> aura). Don't bother with this sort of stuff. >>> >>> >>> I assume so toolkit_views is not going to be supported? >> >> That's right. Once we cut to aura we'll remove all the views+gtk code. >> >>> Which are the flags >>> that >>> are supposed to work on Linux to compile and run views_examples_exe? >> >> use_aura=1 . Most of us are primarily using use_aura=1 and chromeos=1. >> > > views_examples_exe seems to be already broken on Linux with use_aura=1. > > ../../ui/views/examples/examples_main.cc: In function ‘int main(int, char**)’: > ../../ui/views/examples/examples_main.cc:26:15: error: ‘g_type_init’ > was not declared in this scope > ../../ui/views/examples/examples_main.cc:27:24: error: ‘gtk_init’ was > not declared in this scope > >> -Scott > > > > -- > Thiago
lgtm I think we still need this CL according to this bug: http://code.google.com/p/chromium/issues/detail?id=113010 sky can we check this in for the chromeos_gtk use? thanx, alice
LGTM I agree with sky we should not spend anytime on this generally speaking. However it seems that until we are in the clear with our R18 branch, there is some productivity value for folks to be able to build at least "chrome" with ChromeOS+GTK in ToT.
Let me clarify. We're still using views-gtk, but plan to switch to aura in the near future. This means we can't ditch views-gtk and it still needs to work. I was worried you were going to spend time porting things or cleaning up views-gtk when we're close to nuking it. This patch isn't either of these, it's fixing a real issue. So, LGTM and sorry for any confusion.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tfarina@chromium.org/9317111/1 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
