Applied review comments from oshima for CL 7850026.

Applied review comments from oshima for CL 7850026.

BUG=none
TEST=none

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

[Emmanuel Saint-loubert-Bié, 2011-09-19 20:16:27]

All your comments have been applied. Thanks for the feedback!

-- Emmanuel

[oshima, 2011-09-19 22:31:03]

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
File chrome/browser/chromeos/frame/panel_browser_view.cc (left):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
chrome/browser/chromeos/frame/panel_browser_view.cc:112: #else
keep this.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
File chrome/browser/chromeos/frame/panel_browser_view.cc (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
chrome/browser/chromeos/frame/panel_browser_view.cc:36: GdkScreen* screen =
gtk_widget_get_screen(GetWidget()->GetNativeView());
What I meant was that you could comment out the entire method body.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
File chrome/browser/chromeos/frame/panel_controller.cc (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
chrome/browser/chromeos/frame/panel_controller.cc:189:
title_window_->Init(params);
#if !defined(USE_AURA)

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
chrome/browser/chromeos/frame/panel_controller.cc:212: 
#end

exclude OnPanelClientEvent for AURA.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
chrome/browser/chromeos/frame/panel_controller.cc:267: #if !defined(TOUCH_UI)
USE_ONLY_PURE_VIEWS (or TOOLKIT_USES_GTK after sadrul's change)

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/background_view.cc (left):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/background_view.cc:135: #else
would it be possible to compile & link only webui version for aura?

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/idle_linux.cc
File chrome/browser/idle_linux.cc (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/idle_linux.cc...
chrome/browser/idle_linux.cc:29: // TODO(saintlou): We have no Screensaver in
Aura.
we will have, just different implementation. We just need aura version of
ScreenSaverWindowFinder (which can be an empty for now)

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/process_singl...
File chrome/browser/process_singleton.h (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/process_singl...
chrome/browser/process_singleton.h:171: // not in process_singleton_aura.cc.
why we don't need it on linux/aura?

http://codereview.chromium.org/7945014/diff/2001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/chrome_browser.gypi#n...
chrome/chrome_browser.gypi:4982: ['exclude',
'^browser/screensaver_window_finder_linux.h'],
we should rename this to _gtk.cc and have stub _aura file.

[Emmanuel Saint-loubert-Bié, 2011-09-19 23:26:02]

Thanks for the feedback Oshima. PTAL.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
File chrome/browser/chromeos/frame/panel_browser_view.cc (left):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
chrome/browser/chromeos/frame/panel_browser_view.cc:112: #else
On 2011/09/19 22:31:03, oshima wrote:
> keep this.

Done.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
File chrome/browser/chromeos/frame/panel_browser_view.cc (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
chrome/browser/chromeos/frame/panel_browser_view.cc:36: GdkScreen* screen =
gtk_widget_get_screen(GetWidget()->GetNativeView());
On 2011/09/19 22:31:03, oshima wrote:
> What I meant was that you could comment out the entire method body.

Done.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
File chrome/browser/chromeos/frame/panel_controller.cc (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
chrome/browser/chromeos/frame/panel_controller.cc:189:
title_window_->Init(params);
On 2011/09/19 22:31:03, oshima wrote:
> #if !defined(USE_AURA)

Done.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/fram...
chrome/browser/chromeos/frame/panel_controller.cc:212: 
On 2011/09/19 22:31:03, oshima wrote:
> #end
> 
> exclude OnPanelClientEvent for AURA.

Done.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/background_view.cc (left):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/background_view.cc:135: #else
That would be the goal. I am not sure we are ready yet.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/idle_linux.cc
File chrome/browser/idle_linux.cc (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/idle_linux.cc...
chrome/browser/idle_linux.cc:29: // TODO(saintlou): We have no Screensaver in
Aura.
On 2011/09/19 22:31:03, oshima wrote:
> we will have, just different implementation. We just need aura version of
> ScreenSaverWindowFinder (which can be an empty for now)

Done.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/process_singl...
File chrome/browser/process_singleton.h (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/process_singl...
chrome/browser/process_singleton.h:171: // not in process_singleton_aura.cc.
We probably need it eventually but process_singleton_aura.cc is just a stub.

http://codereview.chromium.org/7945014/diff/2001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/chrome_browser.gypi#n...
chrome/chrome_browser.gypi:4982: ['exclude',
'^browser/screensaver_window_finder_linux.h'],
On 2011/09/19 22:31:03, oshima wrote:
> we should rename this to _gtk.cc and have stub _aura file.

Done.

[oshima, 2011-09-20 04:59:17]

Sorry ScreensaverWindowFinder is my mistake. Please see my comments below.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/background_view.cc (left):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/background_view.cc:135: #else
On 2011/09/19 23:26:02, Emmanuel Saint-loubert wrote:
> That would be the goal. I am not sure we are ready yet.

webui oobe/login has been enabled for chromeos, so that should be possible.
screen locker can be simply removed for now.

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/process_singl...
File chrome/browser/process_singleton.h (right):

http://codereview.chromium.org/7945014/diff/2001/chrome/browser/process_singl...
chrome/browser/process_singleton.h:171: // not in process_singleton_aura.cc.
On 2011/09/19 23:26:02, Emmanuel Saint-loubert wrote:
> We probably need it eventually but process_singleton_aura.cc is just a stub.
> 
i see. thanks.

http://codereview.chromium.org/7945014/diff/4018/chrome/browser/idle_linux.cc
File chrome/browser/idle_linux.cc (right):

http://codereview.chromium.org/7945014/diff/4018/chrome/browser/idle_linux.cc...
chrome/browser/idle_linux.cc:13: #include
"chrome/browser/screensaver_window_finder_gtk.h"
Doh, I was stupid. I thought ScreensaverWindowFinder is a shared class. Looks
like this is gtk only. My bad. You may revert this, and simply
return false in "bool CheckIdleStateIsLocked" and rename
screensaver_window_filter_linux.cc to gtk.cc. Sorry about that.

[Emmanuel Saint-loubert-Bié, 2011-09-20 16:25:32]

No pb Oshima! PTAL. Thanks,

-- E

[oshima, 2011-09-20 17:09:20]

LGTM

[commit-bot: I haz the power, 2011-09-20 18:06:39]

Can't apply patch for file chrome/browser/screensaver_window_finder_gtk.cc.
While running patch -p1 --forward --force;
patching file chrome/browser/screensaver_window_finder_gtk.cc
Hunk #1 FAILED at 2.
Hunk #2 FAILED at 17.
2 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/screensaver_window_finder_gtk.cc.rej