[SystemInfo API] Implement DisplayInfoProvider::UpdateDisplayUnitInfoForPlatform for Linux GTK

[SystemInfo API] Implement DisplayInfoProvider::UpdateDisplayUnitInfoForPlatform method for Linux GTK.

Also rename display_info_provider_x11 to display_info_provider_linux for
better understand.

BUG=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221381

[Haojian Wu, 2013-08-26 11:35:05]

This patch will replace the "Implement system.display.getInfo API on linux"
CL(https://codereview.chromium.org/22802002/) since DisplayInfoProvider has been
refactored. And should get landed after "Implement gfx::Screen::GetAllDisplays()
for Linux."(https://codereview.chromium.org/23401004/) CL gets in.

Please review it. Thanks.

[Haojian Wu, 2013-08-26 11:58:36]

https://codereview.chromium.org/22891026/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/system_display/display_info_provider_gtk.cc
(right):

https://codereview.chromium.org/22891026/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/system_display/display_info_provider_gtk.cc:26: //
TODO(Haojian): determine the DPI of the display
oshima@, I find a strange problem while calculating DPI for monitor.
I plan to calculate in the following way:
  dpi = N * 25.4 pixels / M inch

I use gdk_screen_get_monitor_height/width_mm methods to get height/width in
millimeters. But the result is always 0 which I think is not correct. I have
tested on ubuntu 12.04 with gnome/xfce, both of them show the same result 0.

Do you have any idea about this? Thanks.

[oshima, 2013-08-26 20:27:57]

this change lgtm

https://codereview.chromium.org/22891026/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/system_display/display_info_provider_gtk.cc
(right):

https://codereview.chromium.org/22891026/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/system_display/display_info_provider_gtk.cc:26: //
TODO(Haojian): determine the DPI of the display
On 2013/08/26 11:58:36, Haojian Wu wrote:
> oshima@, I find a strange problem while calculating DPI for monitor.
> I plan to calculate in the following way:
>   dpi = N * 25.4 pixels / M inch
> 
> I use gdk_screen_get_monitor_height/width_mm methods to get height/width in
> millimeters. But the result is always 0 which I think is not correct. I have
> tested on ubuntu 12.04 with gnome/xfce, both of them show the same result 0.
> 
> Do you have any idea about this? Thanks.  

Monitor's info may or may not be available. Can you check what you get with
xrandr?

[Haojian Wu, 2013-08-27 01:35:48]

https://codereview.chromium.org/22891026/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/system_display/display_info_provider_gtk.cc
(right):

https://codereview.chromium.org/22891026/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/system_display/display_info_provider_gtk.cc:26: //
TODO(Haojian): determine the DPI of the display
On 2013/08/26 20:27:57, oshima wrote:
> On 2013/08/26 11:58:36, Haojian Wu wrote:
> > oshima@, I find a strange problem while calculating DPI for monitor.
> > I plan to calculate in the following way:
> >   dpi = N * 25.4 pixels / M inch
> > 
> > I use gdk_screen_get_monitor_height/width_mm methods to get height/width in
> > millimeters. But the result is always 0 which I think is not correct. I have
> > tested on ubuntu 12.04 with gnome/xfce, both of them show the same result 0.
> > 
> > Do you have any idea about this? Thanks.  
> 
> Monitor's info may or may not be available. Can you check what you get with
> xrandr?

Running xrandr command, the output also shows 0mm*0mm.

[oshima, 2013-08-27 06:20:17]

On 2013/08/27 01:35:48, Haojian Wu wrote:
>
https://codereview.chromium.org/22891026/diff/1/chrome/browser/extensions/api...
> File chrome/browser/extensions/api/system_display/display_info_provider_gtk.cc
> (right):
> 
>
https://codereview.chromium.org/22891026/diff/1/chrome/browser/extensions/api...
> chrome/browser/extensions/api/system_display/display_info_provider_gtk.cc:26:
//
> TODO(Haojian): determine the DPI of the display
> On 2013/08/26 20:27:57, oshima wrote:
> > On 2013/08/26 11:58:36, Haojian Wu wrote:
> > > oshima@, I find a strange problem while calculating DPI for monitor.
> > > I plan to calculate in the following way:
> > >   dpi = N * 25.4 pixels / M inch
> > > 
> > > I use gdk_screen_get_monitor_height/width_mm methods to get height/width
in
> > > millimeters. But the result is always 0 which I think is not correct. I
have
> > > tested on ubuntu 12.04 with gnome/xfce, both of them show the same result
0.
> > > 
> > > Do you have any idea about this? Thanks.  
> > 
> > Monitor's info may or may not be available. Can you check what you get with
> > xrandr?
> 
> Running xrandr command, the output also shows 0mm*0mm.

That means the info isn't available. Just so you know, DPI information isn't
reliable
and can be bogus or even not applicable (projector for example). That's why
we're
not relying on exact DPI info.

[Hongbo Min, 2013-08-27 06:38:50]

lgtm

[commit-bot: I haz the power, 2013-09-02 00:41:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Hokein.Wu@gmail.com/22891026/1

[commit-bot: I haz the power, 2013-09-02 01:17:09]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[Haojian Wu, 2013-09-02 09:08:11]

https://chromiumcodereview.appspot.com/22891026/diff/32001/chrome/browser/ext...
File chrome/browser/extensions/api/system_display/display_info_provider_linux.cc
(right):

https://chromiumcodereview.appspot.com/22891026/diff/32001/chrome/browser/ext...
chrome/browser/extensions/api/system_display/display_info_provider_linux.cc:5:
#include "chrome/browser/extensions/api/system_display/display_info_provider.h"
Refer to the linux_aura buildbot error stdio log, it looks like
display_info_provider_gtk.cc doesn't get included while building with use_aura=1
on linux.

I have tested on my local machine and reproduce this error. It seems cause by
GYP generated rules, I have little idea on solving this, but rename from gtk
postfix to linux postfix is an optional solution here.

oshima@, any idea about this?

https://chromiumcodereview.appspot.com/22891026/diff/32001/chrome/chrome_brow...
File chrome/chrome_browser_extensions.gypi (right):

https://chromiumcodereview.appspot.com/22891026/diff/32001/chrome/chrome_brow...
chrome/chrome_browser_extensions.gypi:917: '../build/linux/system.gyp:gtk',
@oshima, since we should gtk API in DisplayInfoProvider, this will make
browser_extensions lib depend on gtk library. Is this expected?

[oshima, 2013-09-03 17:14:04]

https://chromiumcodereview.appspot.com/22891026/diff/32001/chrome/browser/ext...
File chrome/browser/extensions/api/system_display/display_info_provider_linux.cc
(right):

https://chromiumcodereview.appspot.com/22891026/diff/32001/chrome/browser/ext...
chrome/browser/extensions/api/system_display/display_info_provider_linux.cc:5:
#include "chrome/browser/extensions/api/system_display/display_info_provider.h"
On 2013/09/02 09:08:12, Haojian Wu wrote:
> Refer to the linux_aura buildbot error stdio log, it looks like
> display_info_provider_gtk.cc doesn't get included while building with
use_aura=1
> on linux.
> 
> I have tested on my local machine and reproduce this error. It seems cause by
> GYP generated rules, I have little idea on solving this, but rename from gtk
> postfix to linux postfix is an optional solution here.
> 
> oshima@, any idea about this?
this should be renamed to dispaly_info_provider_gtk.cc as this is for gtk/gtk
impl.

https://chromiumcodereview.appspot.com/22891026/diff/32001/chrome/chrome_brow...
File chrome/chrome_browser_extensions.gypi (right):

https://chromiumcodereview.appspot.com/22891026/diff/32001/chrome/chrome_brow...
chrome/chrome_browser_extensions.gypi:917: '../build/linux/system.gyp:gtk',
no please do not add gtk dependency to linux_aura

[Hongbo Min, 2013-09-04 01:46:04]

https://codereview.chromium.org/22891026/diff/32001/chrome/browser/extensions...
File chrome/browser/extensions/api/system_display/display_info_provider_linux.cc
(right):

https://codereview.chromium.org/22891026/diff/32001/chrome/browser/extensions...
chrome/browser/extensions/api/system_display/display_info_provider_linux.cc:20:
base::Bind(callback, false, "234 implemented"));
what is the '234' meaning here?

[Haojian Wu, 2013-09-04 15:02:56]

https://codereview.chromium.org/22891026/diff/32001/chrome/browser/extensions...
File chrome/browser/extensions/api/system_display/display_info_provider_linux.cc
(right):

https://codereview.chromium.org/22891026/diff/32001/chrome/browser/extensions...
chrome/browser/extensions/api/system_display/display_info_provider_linux.cc:5:
#include "chrome/browser/extensions/api/system_display/display_info_provider.h"
On 2013/09/03 17:14:05, oshima wrote:
> On 2013/09/02 09:08:12, Haojian Wu wrote:
> > Refer to the linux_aura buildbot error stdio log, it looks like
> > display_info_provider_gtk.cc doesn't get included while building with
> use_aura=1
> > on linux.
> > 
> > I have tested on my local machine and reproduce this error. It seems cause
by
> > GYP generated rules, I have little idea on solving this, but rename from gtk
> > postfix to linux postfix is an optional solution here.
> > 
> > oshima@, any idea about this?
> this should be renamed to dispaly_info_provider_gtk.cc as this is for gtk/gtk
> impl.
> 

Done.

https://codereview.chromium.org/22891026/diff/32001/chrome/browser/extensions...
chrome/browser/extensions/api/system_display/display_info_provider_linux.cc:20:
base::Bind(callback, false, "234 implemented"));
On 2013/09/04 01:46:04, Hongbo Min wrote:
> what is the '234' meaning here?

Just a typo error. Done.

https://codereview.chromium.org/22891026/diff/32001/chrome/chrome_browser_ext...
File chrome/chrome_browser_extensions.gypi (right):

https://codereview.chromium.org/22891026/diff/32001/chrome/chrome_browser_ext...
chrome/chrome_browser_extensions.gypi:917: '../build/linux/system.gyp:gtk',
On 2013/09/03 17:14:05, oshima wrote:
> no please do not add gtk dependency to linux_aura

Done.

https://codereview.chromium.org/22891026/diff/43001/chrome/browser/extensions...
File chrome/browser/extensions/api/system_display/display_info_provider_aura.cc
(right):

https://codereview.chromium.org/22891026/diff/43001/chrome/browser/extensions...
chrome/browser/extensions/api/system_display/display_info_provider_aura.cc:6: 
It should add a new file for use_aura build to avoid build error.

Please review it again.

[oshima, 2013-09-04 17:13:40]

https://codereview.chromium.org/22891026/diff/43001/chrome/chrome_browser_ext...
File chrome/chrome_browser_extensions.gypi (right):

https://codereview.chromium.org/22891026/diff/43001/chrome/chrome_browser_ext...
chrome/chrome_browser_extensions.gypi:487:
'browser/extensions/api/system_display/display_info_provider_chromeos.cc',
chromeos builds both _aura and _chromeos.cc, so you most likely
need to exclude display_info_provider_aura.cc for chromeos build.

[Haojian Wu, 2013-09-05 02:41:04]

https://codereview.chromium.org/22891026/diff/43001/chrome/chrome_browser_ext...
File chrome/chrome_browser_extensions.gypi (right):

https://codereview.chromium.org/22891026/diff/43001/chrome/chrome_browser_ext...
chrome/chrome_browser_extensions.gypi:487:
'browser/extensions/api/system_display/display_info_provider_chromeos.cc',
On 2013/09/04 17:13:40, oshima wrote:
> chromeos builds both _aura and _chromeos.cc, so you most likely
> need to exclude display_info_provider_aura.cc for chromeos build.

Done.

[oshima, 2013-09-05 07:16:38]

lgtm

[Hongbo Min, 2013-09-05 08:41:05]

lgtm

[commit-bot: I haz the power, 2013-09-05 08:49:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Hokein.Wu@gmail.com/22891026/31001

[commit-bot: I haz the power, 2013-09-05 11:17:27]

Change committed as 221381