Aura under Linux

Aura under Linux is enabled with:

   'use_aura': 1,
 

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=101704

[Emmanuel Saint-loubert-Bié, 2011-09-14 21:46:37]

This is not in review, just FYI. 

Just synced'up with Ben latest use_aura changed in .gypi. 
Will try to finish resolving link issues today.

-- E

[Emmanuel Saint-loubert-Bié, 2011-09-15 02:10:25]

I am down to the last few link errors, but not done. There is a lot of GTKiness
everywhere. Any I updated the CL if you are interested in progress and/or have
ideas/comments.

http://codereview.chromium.org/7850026/diff/8001/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (left):

http://codereview.chromium.org/7850026/diff/8001/chrome/browser/ui/browser.cc...
chrome/browser/ui/browser.cc:1947: #else
The code above was never compiled, if it did then it would have failed since the
FileBrowserUI class is not defined anywhere

[Ben Goodger (Google), 2011-09-16 01:11:18]

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/gtk/gtk_u...
File chrome/browser/ui/gtk/gtk_util.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/gtk/gtk_u...
chrome/browser/ui/gtk/gtk_util.cc:193: // TODO(saintlou): This entire module
should be removed from Aura soon
Why do you need to build this file?

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/tabs/dock...
File chrome/browser/ui/tabs/dock_info_aura.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/tabs/dock...
chrome/browser/ui/tabs/dock_info_aura.cc:9: #endif
Platform ifdefs go after the cross platform ones.

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/bub...
File chrome/browser/ui/views/bubble/bubble.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/bub...
chrome/browser/ui/views/bubble/bubble.cc:114: (void)opacity;
que?

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/htm...
File chrome/browser/ui/views/html_dialog_view.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/htm...
chrome/browser/ui/views/html_dialog_view.cc:215: #elif defined(TOOLKIT_USES_GTK)
&& !defined(USE_AURA)
In these circumstances I prefer the following flow:

#if defined(USE_AURA)
..
#elif defined(OS_WIN)
..
#elif defined(TOOLKIT_USES_GTK)
..
#endif

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/not...
File chrome/browser/ui/views/notifications/balloon_view_host.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/not...
chrome/browser/ui/views/notifications/balloon_view_host.cc:15: #include
"chrome/browser/renderer_host/render_widget_host_view_views.h"
Again, the flow here should read:

#if defined(USE_AURA) <-- always first, since it applies to all platforms
..
#elif defined(OS_WIN)
..
#elif defined(TOOLKIT_USES_GTK)
#if defined(TOUCH_UI)
..
#else
..
#endif
#endif

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/stu...
File chrome/browser/ui/views/stubs_aura.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/stu...
chrome/browser/ui/views/stubs_aura.cc:15: #if defined(USE_NSS)
alphabetically sort platform-specific #defines where possible.. i.e. this
section after OS_WINDOWS

http://codereview.chromium.org/7850026/diff/12001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/chrome_browser.gypi#...
chrome/chrome_browser.gypi:4885: ['use_aura==0', {
This is not inside any other conditional now, is it?

http://codereview.chromium.org/7850026/diff/12001/content/browser/renderer_ho...
File content/browser/renderer_host/render_message_filter_aura.cc (right):

http://codereview.chromium.org/7850026/diff/12001/content/browser/renderer_ho...
content/browser/renderer_host/render_message_filter_aura.cc:9: #if
defined(OS_WINDOWS)
I believe it's OS_WIN

http://codereview.chromium.org/7850026/diff/12001/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host.cc (right):

http://codereview.chromium.org/7850026/diff/12001/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host.cc:207: #if
defined(TOOLKIT_USES_GTK) && !defined(USE_AURA)
TOOLKIT_USES_GTK should never be defined with USE_AURA, so this extra check
should not be necessary.

http://codereview.chromium.org/7850026/diff/12001/ui/ui.gyp
File ui/ui.gyp (right):

http://codereview.chromium.org/7850026/diff/12001/ui/ui.gyp#newcode305
ui/ui.gyp:305: ['include', 'gfx/gtk_util.cc'],
why?

[Emmanuel Saint-loubert-Bié, 2011-09-16 02:34:54]

Hi All, 

This builds "on my machine" for Linux with/without use_aura, with/without
touch_ui.

We will see what the trybots have to say!

-- Emmanuel

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/gtk/gtk_u...
File chrome/browser/ui/gtk/gtk_util.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/gtk/gtk_u...
chrome/browser/ui/gtk/gtk_util.cc:193: // TODO(saintlou): This entire module
should be removed from Aura soon
There are a few methods that we need, but will be gone post-GTK. We could also
break them up.

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/tabs/dock...
File chrome/browser/ui/tabs/dock_info_aura.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/tabs/dock...
chrome/browser/ui/tabs/dock_info_aura.cc:9: #endif
Done.

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/bub...
File chrome/browser/ui/views/bubble/bubble.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/bub...
chrome/browser/ui/views/bubble/bubble.cc:114: (void)opacity;
without the (void) you get a warning that you have an unused variable and since
warning are treated as errors this is the way I handled those in a prior life.
Let me know if you handle this differently in Chrome.
(BTW this suggest that Windows does not have the same compilation flag enabled).

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/htm...
File chrome/browser/ui/views/html_dialog_view.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/htm...
chrome/browser/ui/views/html_dialog_view.cc:215: #elif defined(TOOLKIT_USES_GTK)
&& !defined(USE_AURA)
Yes much cleaner!

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/stu...
File chrome/browser/ui/views/stubs_aura.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/stu...
chrome/browser/ui/views/stubs_aura.cc:15: #if defined(USE_NSS)
Done.

http://codereview.chromium.org/7850026/diff/12001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/chrome_browser.gypi#...
chrome/chrome_browser.gypi:4885: ['use_aura==0', {
Should it? I actually reworked this entire file to make it more friendly with
Aura on Win definitions.

http://codereview.chromium.org/7850026/diff/12001/content/browser/renderer_ho...
File content/browser/renderer_host/render_message_filter_aura.cc (right):

http://codereview.chromium.org/7850026/diff/12001/content/browser/renderer_ho...
content/browser/renderer_host/render_message_filter_aura.cc:9: #if
defined(OS_WINDOWS)
Good catch. I fixed it everywhere. Thanks.

http://codereview.chromium.org/7850026/diff/12001/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host.cc (right):

http://codereview.chromium.org/7850026/diff/12001/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host.cc:207: #if
defined(TOOLKIT_USES_GTK) && !defined(USE_AURA)
Unfortunately even under PureViews we might need to run some GTK. I will redo a
pass for these later on.

http://codereview.chromium.org/7850026/diff/12001/ui/ui.gyp
File ui/ui.gyp (right):

http://codereview.chromium.org/7850026/diff/12001/ui/ui.gyp#newcode305
ui/ui.gyp:305: ['include', 'gfx/gtk_util.cc'],
We still need under Linux. In any case it was wrong of me to include this for
Windows too.

[Ben Goodger (Google), 2011-09-16 14:46:55]

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/bub...
File chrome/browser/ui/views/bubble/bubble.cc (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/browser/ui/views/bub...
chrome/browser/ui/views/bubble/bubble.cc:114: (void)opacity;
Huh. OK.

http://codereview.chromium.org/7850026/diff/12001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/7850026/diff/12001/chrome/chrome_browser.gypi#...
chrome/chrome_browser.gypi:4885: ['use_aura==0', {
Nope, just checking!

http://codereview.chromium.org/7850026/diff/12001/ui/ui.gyp
File ui/ui.gyp (right):

http://codereview.chromium.org/7850026/diff/12001/ui/ui.gyp#newcode305
ui/ui.gyp:305: ['include', 'gfx/gtk_util.cc'],
Can you let me know of the specific instances? I'd like to track them.

[Emmanuel Saint-loubert-Bié, 2011-09-16 17:32:23]

Hi Ben,

Regarding:

> http://codereview.chromium.org/7850026/diff/12001/ui/ui.gyp#newcode305
> ui/ui.gyp:305: ['include', 'gfx/gtk_util.cc'],
> Can you let me know of the specific instances? I'd like to track them.

This is specifically for these 3 methods:

gfx::GdkPixbufFromSkBitmap(SkBitmap const*)
gfx::GetCursor(int)
gfx::GtkInitFromCommandLine(CommandLine const&)

How would like to handle these?

-- Emmanuel

[Ben Goodger (Google), 2011-09-16 17:49:02]

On Fri, Sep 16, 2011 at 10:32 AM, Emmanuel Saint-loubert <
saintlou@chromium.org> wrote:

> Hi Ben,
>
> Regarding:
>
> > http://codereview.chromium.org/7850026/diff/12001/ui/ui.gyp#newcode305
> > ui/ui.gyp:305: ['include', 'gfx/gtk_util.cc'],
> > Can you let me know of the specific instances? I'd like to track them.
>
> This is specifically for these 3 methods:
>
> gfx::GdkPixbufFromSkBitmap(SkBitmap const*)
> gfx::GetCursor(int)
> gfx::GtkInitFromCommandLine(CommandLine const&)
>
> How would like to handle these?
>

What are they used for, in particular?

-Ben

[Emmanuel Saint-loubert-Bié, 2011-09-16 18:43:34]

Ben

I addressed the gtk_utils issues. PTAL.

-- E

[Ben Goodger (Google), 2011-09-16 19:59:26]

LGTM

[sadrul, 2011-09-16 21:11:57]

LGTM

http://codereview.chromium.org/7850026/diff/14104/chrome/browser/renderer_pre...
File chrome/browser/renderer_preferences_util.cc (right):

http://codereview.chromium.org/7850026/diff/14104/chrome/browser/renderer_pre...
chrome/browser/renderer_preferences_util.cc:22: #if !defined(OS_CHROMEOS) &&
!defined(USE_AURA)
This change isn't necessary?

http://codereview.chromium.org/7850026/diff/14104/chrome/common/render_messag...
File chrome/common/render_messages.h (right):

http://codereview.chromium.org/7850026/diff/14104/chrome/common/render_messag...
chrome/common/render_messages.h:78: #endif  // defined(OS_POSIX)
Here too!

[Emmanuel Saint-loubert-Bié, 2011-09-16 22:55:01]

Thanks all. I am planning to land this Sunday or Monday first thing.

-- Emmanuel

http://codereview.chromium.org/7850026/diff/14104/chrome/browser/renderer_pre...
File chrome/browser/renderer_preferences_util.cc (right):

http://codereview.chromium.org/7850026/diff/14104/chrome/browser/renderer_pre...
chrome/browser/renderer_preferences_util.cc:22: #if !defined(OS_CHROMEOS) &&
!defined(USE_AURA)
If we build the browser without ChromeOS and with use_aura we do not want the
GTK-ness below. I do not think we can change this.

http://codereview.chromium.org/7850026/diff/14104/chrome/common/render_messag...
File chrome/common/render_messages.h (right):

http://codereview.chromium.org/7850026/diff/14104/chrome/common/render_messag...
chrome/common/render_messages.h:78: #endif  // defined(OS_POSIX)
On 2011/09/16 21:11:58, sadrul wrote:
> Here too!

Done.

[commit-bot: I haz the power, 2011-09-18 17:25:06]

Presubmit check for 7850026-22073 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
Found a tab character in:

***************
views/focus/accelerator_handler_aura.cc, line 24
views/focus/accelerator_handler_aura.cc, line 25
***************

Found line ending with white spaces in:

***************
views/focus/accelerator_handler_aura.cc, line 24
views/focus/accelerator_handler_aura.cc, line 25
***************

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
chrome/browser/profiles/profile.cc,content/plugin/plugin_thread.cc

Presubmit checks took 8.4s to calculate.

[oshima, 2011-09-19 15:52:38]

Sorry for late review. Can you address them in follow up CL?

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
File chrome/browser/chromeos/frame/bubble_window_views.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
chrome/browser/chromeos/frame/bubble_window_views.cc:23: // TODO(saintlou): Once
Views are truly pure the code below needs to be
indent

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
File chrome/browser/chromeos/frame/panel_browser_view.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
chrome/browser/chromeos/frame/panel_browser_view.cc:37: // TODO(saintlou): Need
PureViews panels?
On chromeos, this will be controlled by WM so this may not be necessary.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
File chrome/browser/chromeos/frame/panel_controller.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
chrome/browser/chromeos/frame/panel_controller.cc:183: // TODO(saintlou): Need
PureView for panels.
You can just skip gtk/WM stuff for now. The result should be replaced by aura WM
API.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/background_view.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/background_view.cc:133: // TODO(saintlou):
I believe this will be gone

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/helper.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/helper.cc:85: // TODO(saintlou): Is this still in
use with the new WebUI anyway?
this will be gone.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/process_sing...
File chrome/browser/process_singleton.h (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/process_sing...
chrome/browser/process_singleton.h:170: class LinuxWatcher;
Why this has to be removed? TODO would be nice.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/screensaver_...
File chrome/browser/screensaver_window_finder_linux.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/screensaver_...
chrome/browser/screensaver_window_finder_linux.cc:20: #if defined(USE_AURA)
I think this file should not be linked with aura.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/ui/gtk/ssl_c...
File chrome/browser/ui/gtk/ssl_client_certificate_selector.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/ui/gtk/ssl_c...
chrome/browser/ui/gtk/ssl_client_certificate_selector.cc:350: // TODO(saintlou):
need to port to Views.
q: The plan is to switch to webui, corrrect?

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/ui/views/sel...
File chrome/browser/ui/views/select_file_dialog.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/ui/views/sel...
chrome/browser/ui/views/select_file_dialog.cc:352: // when chromeos==0.
Shouldn't this code be harmless with the empty URL?

http://codereview.chromium.org/7850026/diff/24001/content/browser/browser_mai...
File content/browser/browser_main.cc (right):

http://codereview.chromium.org/7850026/diff/24001/content/browser/browser_mai...
content/browser/browser_main.cc:295: // TODO(saintlou): We still need some GTK
at the lowest level, so init here.
Do you remember what was the issue?

http://codereview.chromium.org/7850026/diff/24001/content/browser/plugin_proc...
File content/browser/plugin_process_host.cc (right):

http://codereview.chromium.org/7850026/diff/24001/content/browser/plugin_proc...
content/browser/plugin_process_host.cc:105: #if !defined(USE_AURA)
I think this method isn't necessary for aura.

http://codereview.chromium.org/7850026/diff/24001/content/browser/renderer_ho...
File content/browser/renderer_host/render_message_filter_aura.cc (right):

http://codereview.chromium.org/7850026/diff/24001/content/browser/renderer_ho...
content/browser/renderer_host/render_message_filter_aura.cc:9: #if
defined(OS_WIN)
id' be better to handle this in gyp fine.

[oshima, 2011-09-19 15:55:48]

forgot to comment on this.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/renderer_hos...
File chrome/browser/renderer_host/render_widget_host_view_views.h (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/renderer_hos...
chrome/browser/renderer_host/render_widget_host_view_views.h:94: virtual void
GetDefaultScreenInfo(WebKit::WebScreenInfo* results) OVERRIDE;
Is this necessary? I wonder how this passed clang build...

[Emmanuel Saint-loubert-Bié, 2011-09-19 16:05:17]

Yes the CL landed yesterday. I will do a follow up CL, no pb.

BTW this mean you should try to land your CL ASAP, then Roy's. Thanks,

-- Emmanuel

On Mon, Sep 19, 2011 at 8:52 AM,  <oshima@chromium.org> wrote:
> Sorry for late review. Can you address them in follow up CL?
>
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
> File chrome/browser/chromeos/frame/bubble_window_views.cc (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
> chrome/browser/chromeos/frame/bubble_window_views.cc:23: //
> TODO(saintlou): Once Views are truly pure the code below needs to be
> indent
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
> File chrome/browser/chromeos/frame/panel_browser_view.cc (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
> chrome/browser/chromeos/frame/panel_browser_view.cc:37: //
> TODO(saintlou): Need PureViews panels?
> On chromeos, this will be controlled by WM so this may not be necessary.
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
> File chrome/browser/chromeos/frame/panel_controller.cc (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
> chrome/browser/chromeos/frame/panel_controller.cc:183: //
> TODO(saintlou): Need PureView for panels.
> You can just skip gtk/WM stuff for now. The result should be replaced by
> aura WM API.
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
> File chrome/browser/chromeos/login/background_view.cc (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
> chrome/browser/chromeos/login/background_view.cc:133: // TODO(saintlou):
> I believe this will be gone
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
> File chrome/browser/chromeos/login/helper.cc (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
> chrome/browser/chromeos/login/helper.cc:85: // TODO(saintlou): Is this
> still in use with the new WebUI anyway?
> this will be gone.
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/process_sing...
> File chrome/browser/process_singleton.h (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/process_sing...
> chrome/browser/process_singleton.h:170: class LinuxWatcher;
> Why this has to be removed? TODO would be nice.
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/screensaver_...
> File chrome/browser/screensaver_window_finder_linux.cc (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/screensaver_...
> chrome/browser/screensaver_window_finder_linux.cc:20: #if
> defined(USE_AURA)
> I think this file should not be linked with aura.
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/ui/gtk/ssl_c...
> File chrome/browser/ui/gtk/ssl_client_certificate_selector.cc (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/ui/gtk/ssl_c...
> chrome/browser/ui/gtk/ssl_client_certificate_selector.cc:350: //
> TODO(saintlou): need to port to Views.
> q: The plan is to switch to webui, corrrect?
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/ui/views/sel...
> File chrome/browser/ui/views/select_file_dialog.cc (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/chrome/browser/ui/views/sel...
> chrome/browser/ui/views/select_file_dialog.cc:352: // when chromeos==0.
> Shouldn't this code be harmless with the empty URL?
>
>
http://codereview.chromium.org/7850026/diff/24001/content/browser/browser_mai...
> File content/browser/browser_main.cc (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/content/browser/browser_mai...
> content/browser/browser_main.cc:295: // TODO(saintlou): We still need
> some GTK at the lowest level, so init here.
> Do you remember what was the issue?
>
>
http://codereview.chromium.org/7850026/diff/24001/content/browser/plugin_proc...
> File content/browser/plugin_process_host.cc (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/content/browser/plugin_proc...
> content/browser/plugin_process_host.cc:105: #if !defined(USE_AURA)
> I think this method isn't necessary for aura.
>
>
http://codereview.chromium.org/7850026/diff/24001/content/browser/renderer_ho...
> File content/browser/renderer_host/render_message_filter_aura.cc
> (right):
>
>
http://codereview.chromium.org/7850026/diff/24001/content/browser/renderer_ho...
> content/browser/renderer_host/render_message_filter_aura.cc:9: #if
> defined(OS_WIN)
> id' be better to handle this in gyp fine.
>
> http://codereview.chromium.org/7850026/
>

[Emmanuel Saint-loubert-Bié, 2011-09-19 20:15:36]

All of Oshima's comments addressed in http://codereview.chromium.org/7945014

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
File chrome/browser/chromeos/frame/bubble_window_views.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
chrome/browser/chromeos/frame/bubble_window_views.cc:23: // TODO(saintlou): Once
Views are truly pure the code below needs to be
On 2011/09/19 15:52:39, oshima wrote:
> indent

Done.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
File chrome/browser/chromeos/frame/panel_browser_view.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
chrome/browser/chromeos/frame/panel_browser_view.cc:37: // TODO(saintlou): Need
PureViews panels?
On 2011/09/19 15:52:39, oshima wrote:
> On chromeos, this will be controlled by WM so this may not be necessary.

Done.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
File chrome/browser/chromeos/frame/panel_controller.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/fra...
chrome/browser/chromeos/frame/panel_controller.cc:183: // TODO(saintlou): Need
PureView for panels.
On 2011/09/19 15:52:39, oshima wrote:
> You can just skip gtk/WM stuff for now. The result should be replaced by aura
WM
> API.

Done.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/background_view.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/background_view.cc:133: // TODO(saintlou):
On 2011/09/19 15:52:39, oshima wrote:
> I believe this will be gone

Done.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/helper.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/helper.cc:85: // TODO(saintlou): Is this still in
use with the new WebUI anyway?
On 2011/09/19 15:52:39, oshima wrote:
> this will be gone.

Done.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/process_sing...
File chrome/browser/process_singleton.h (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/process_sing...
chrome/browser/process_singleton.h:170: class LinuxWatcher;
On 2011/09/19 15:52:39, oshima wrote:
> Why this has to be removed? TODO would be nice.

Done.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/renderer_hos...
File chrome/browser/renderer_host/render_widget_host_view_views.h (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/renderer_hos...
chrome/browser/renderer_host/render_widget_host_view_views.h:94: virtual void
GetDefaultScreenInfo(WebKit::WebScreenInfo* results) OVERRIDE;
No it did not pass Clang. I had to fix that with a subsequent query. Should I
remove this method altogether?

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/screensaver_...
File chrome/browser/screensaver_window_finder_linux.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/screensaver_...
chrome/browser/screensaver_window_finder_linux.cc:20: #if defined(USE_AURA)
On 2011/09/19 15:52:39, oshima wrote:
> I think this file should not be linked with aura.

Done.

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/ui/gtk/ssl_c...
File chrome/browser/ui/gtk/ssl_client_certificate_selector.cc (right):

http://codereview.chromium.org/7850026/diff/24001/chrome/browser/ui/gtk/ssl_c...
chrome/browser/ui/gtk/ssl_client_certificate_selector.cc:350: // TODO(saintlou):
need to port to Views.
Yes it is on rbyers list.