chromeos: Choose monitor native mode as best match mode

chromeos: Choose monitor native mode as best match mode

Previously the UI chooses the highest pixel rate mode when multiple
modes exists for the same screen resolution.  If there exist a higher
pixel clock mode that's the same as the monitor's preferred mode, then
HDMI is first plugged in it will choose the preferred mode, but when
you later switch away to a different resolution and then switch back,
you will get the higher pixel clock version.
This patch changes FindDisplayModeMatchingSize() to choose the
preferred mode instead of the one with highest pixel clock.

BUG=chrome-os-partner:29514
TEST=Using the EDID of DELL U2410, there are two 1920x1200 modes
  1920x1200 (0x1d2)  154.0MHz -HSync +VSync *current +preferred
        h: width  1920 start 1968 end 2000 total 2080 skew    0 clock   74.0KHz
        v: height 1200 start 1203 end 1209 total 1235           clock   60.0Hz
  1920x1200 (0x1d3)  193.2MHz -HSync -VSync
        h: width  1920 start 2049 end 2256 total 2593 skew    0 clock   74.5KHz
        v: height 1200 start 1202 end 1205 total 1242           clock   60.0Hz

plug HDMI in, and obseve that it uses mode 0x1d2.
switch to 720p mode, then switch back to 1920x1200
observe that it does not use mode 0x1d3.

Committed: https://crrev.com/6923c2bc95e85e4ef800101a534744c03b25048a
Cr-Commit-Position: refs/heads/master@{#300976}

[davidung, 2014-09-26 00:41:32]

davidu@nvidia.com changed reviewers:
+ derat@chromium.org, dgreid@chromium.org, djkurtz@chromium.org

[davidung, 2014-10-13 21:07:26]

ping

[Daniel Erat, 2014-10-13 23:04:22]

derat@chromium.org changed reviewers:
+ dnicoara@chromium.org, marcheu@chromium.org

[Daniel Erat, 2014-10-13 23:04:23]

Please also add tests for this change to
ui/display/chromeos/display_configurator_unittest.cc.

[davidung, 2014-10-13 23:30:36]

On 2014/10/13 23:04:23, Daniel Erat wrote:
> Please also add tests for this change to
> ui/display/chromeos/display_configurator_unittest.cc.

ok.  will do

[davidung, 2014-10-20 21:56:02]

On 2014/10/13 23:30:36, davidung wrote:
> On 2014/10/13 23:04:23, Daniel Erat wrote:
> > Please also add tests for this change to
> > ui/display/chromeos/display_configurator_unittest.cc.
> 
> ok.  will do

finally getting sometime to look at this.
but I have a simple questions before I start, how do I run the unit tests?

I looked at
http://www.chromium.org/developers/testing/running-tests

but I don't see anything in /out/Debug

$ ls Debug
build.ninja  obj

[Daniel Erat, 2014-10-20 21:59:41]

build the appropriate test binary (display_unittests in this case) and then run
it with --gtest_filter='DisplayConfiguratorTest.*'.

[davidung, 2014-10-20 22:33:47]

On 2014/10/20 21:59:41, Daniel Erat wrote:
> build the appropriate test binary (display_unittests in this case) and then
run
> it with --gtest_filter='DisplayConfiguratorTest.*'.

how do I build the display_unittests?
I am building the browser under the chromeos chroot with

$ CHROME_ORIGIN=LOCAL_SOURCE emerge-nyan chromeos-chrome

$ cd /var/cache/chromeos-chrome/chrome-src/src/out_nyan/Release
$ find ./ -name '*unittests'
./sandbox_linux_unittests
./media_unittests

is there a wiki I should be following?

[Daniel Erat, 2014-10-20 22:38:40]

On 2014/10/20 22:33:47, davidung wrote:
> On 2014/10/20 21:59:41, Daniel Erat wrote:
> > build the appropriate test binary (display_unittests in this case) and then
> run
> > it with --gtest_filter='DisplayConfiguratorTest.*'.
> 
> how do I build the display_unittests?
> I am building the browser under the chromeos chroot with
> 
> $ CHROME_ORIGIN=LOCAL_SOURCE emerge-nyan chromeos-chrome
> 
> $ cd /var/cache/chromeos-chrome/chrome-src/src/out_nyan/Release
> $ find ./ -name '*unittests'
> ./sandbox_linux_unittests
> ./media_unittests
> 
> is there a wiki I should be following?

oh, you shouldn't be using the chroot for this. do a regular non-chroot chrome
build with chromeos=1 set via $GYP_DEFINES. see
https://code.google.com/p/chromium/wiki/LinuxBuildInstructions if you don't know
how to do this.

[davidung, 2014-10-20 23:59:46]

On 2014/10/20 22:38:40, Daniel Erat wrote:
> On 2014/10/20 22:33:47, davidung wrote:
> > On 2014/10/20 21:59:41, Daniel Erat wrote:
> > > build the appropriate test binary (display_unittests in this case) and
then
> > run
> > > it with --gtest_filter='DisplayConfiguratorTest.*'.
> > 
> > how do I build the display_unittests?
> > I am building the browser under the chromeos chroot with
> > 
> > $ CHROME_ORIGIN=LOCAL_SOURCE emerge-nyan chromeos-chrome
> > 
> > $ cd /var/cache/chromeos-chrome/chrome-src/src/out_nyan/Release
> > $ find ./ -name '*unittests'
> > ./sandbox_linux_unittests
> > ./media_unittests
> > 
> > is there a wiki I should be following?
> 
> oh, you shouldn't be using the chroot for this. do a regular non-chroot chrome
> build with chromeos=1 set via $GYP_DEFINES. see
> https://code.google.com/p/chromium/wiki/LinuxBuildInstructions if you don't
know
> how to do this.

ok, thanks
after following http://code.google.com/p/chromium/wiki/LinuxChromiumArm  and
ninja -C out/Debug display_unittests

localhost tmp # ./display_unittests --single-process-tests
[17514:17514:1020/165409:270374390396:ERROR:memory_mapped_file.cc(41)] Couldn't
open /tmp/icudtl.dat
[17514:17514:1020/165409:270374390538:ERROR:icu_util.cc(149)] Couldn't mmap
/tmp/icudtl.dat
[==========] Running 9 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 2 tests from DisplayUtilTest
[ RUN      ] DisplayUtilTest.TestBlackListedDisplay
[       OK ] DisplayUtilTest.TestBlackListedDisplay (0 ms)
[ RUN      ] DisplayUtilTest.GetScaleFactor
[       OK ] DisplayUtilTest.GetScaleFactor (0 ms)
[----------] 2 tests from DisplayUtilTest (0 ms total)

[----------] 7 tests from EDIDParserTest
[ RUN      ] EDIDParserTest.ParseOverscanFlag
[       OK ] EDIDParserTest.ParseOverscanFlag (0 ms)
[ RUN      ] EDIDParserTest.ParseBrokenOverscanData
[       OK ] EDIDParserTest.ParseBrokenOverscanData (0 ms)
[ RUN      ] EDIDParserTest.ParseEDID
[       OK ] EDIDParserTest.ParseEDID (0 ms)
[ RUN      ] EDIDParserTest.ParseBrokenEDID
[17514:17514:1020/165409:270374391256:ERROR:edid_parser.cc(74)] too short EDID
data: manifacturer id
[17514:17514:1020/165409:270374391297:ERROR:edid_parser.cc(116)] invalid EDID:
human unreadable char in name
[       OK ] EDIDParserTest.ParseBrokenEDID (0 ms)
[ RUN      ] EDIDParserTest.GetDisplayId
[       OK ] EDIDParserTest.GetDisplayId (0 ms)
[ RUN      ] EDIDParserTest.GetDisplayIdFromInternal
[       OK ] EDIDParserTest.GetDisplayIdFromInternal (0 ms)
[ RUN      ] EDIDParserTest.GetDisplayIdFailure
[17514:17514:1020/165409:270374391496:ERROR:edid_parser.cc(74)] too short EDID
data: manifacturer id
[       OK ] EDIDParserTest.GetDisplayIdFailure (0 ms)
[----------] 7 tests from EDIDParserTest (0 ms total)

[----------] Global test environment tear-down
[==========] 9 tests from 2 test cases ran. (1 ms total)
[  PASSED  ] 9 tests.

Don't seem to be calling display_configurator_unittest?

[Daniel Erat, 2014-10-21 14:55:11]

On 2014/10/20 23:59:46, davidung wrote:
> On 2014/10/20 22:38:40, Daniel Erat wrote:
> > On 2014/10/20 22:33:47, davidung wrote:
> > > On 2014/10/20 21:59:41, Daniel Erat wrote:
> > > > build the appropriate test binary (display_unittests in this case) and
> then
> > > run
> > > > it with --gtest_filter='DisplayConfiguratorTest.*'.
> > > 
> > > how do I build the display_unittests?
> > > I am building the browser under the chromeos chroot with
> > > 
> > > $ CHROME_ORIGIN=LOCAL_SOURCE emerge-nyan chromeos-chrome
> > > 
> > > $ cd /var/cache/chromeos-chrome/chrome-src/src/out_nyan/Release
> > > $ find ./ -name '*unittests'
> > > ./sandbox_linux_unittests
> > > ./media_unittests
> > > 
> > > is there a wiki I should be following?
> > 
> > oh, you shouldn't be using the chroot for this. do a regular non-chroot
chrome
> > build with chromeos=1 set via $GYP_DEFINES. see
> > https://code.google.com/p/chromium/wiki/LinuxBuildInstructions if you don't
> know
> > how to do this.
> 
> ok, thanks
> after following http://code.google.com/p/chromium/wiki/LinuxChromiumArm  and
> ninja -C out/Debug display_unittests
> 
> localhost tmp # ./display_unittests --single-process-tests
> [17514:17514:1020/165409:270374390396:ERROR:memory_mapped_file.cc(41)]
Couldn't
> open /tmp/icudtl.dat
> [17514:17514:1020/165409:270374390538:ERROR:icu_util.cc(149)] Couldn't mmap
> /tmp/icudtl.dat
> [==========] Running 9 tests from 2 test cases.
> [----------] Global test environment set-up.
> [----------] 2 tests from DisplayUtilTest
> [ RUN      ] DisplayUtilTest.TestBlackListedDisplay
> [       OK ] DisplayUtilTest.TestBlackListedDisplay (0 ms)
> [ RUN      ] DisplayUtilTest.GetScaleFactor
> [       OK ] DisplayUtilTest.GetScaleFactor (0 ms)
> [----------] 2 tests from DisplayUtilTest (0 ms total)
> 
> [----------] 7 tests from EDIDParserTest
> [ RUN      ] EDIDParserTest.ParseOverscanFlag
> [       OK ] EDIDParserTest.ParseOverscanFlag (0 ms)
> [ RUN      ] EDIDParserTest.ParseBrokenOverscanData
> [       OK ] EDIDParserTest.ParseBrokenOverscanData (0 ms)
> [ RUN      ] EDIDParserTest.ParseEDID
> [       OK ] EDIDParserTest.ParseEDID (0 ms)
> [ RUN      ] EDIDParserTest.ParseBrokenEDID
> [17514:17514:1020/165409:270374391256:ERROR:edid_parser.cc(74)] too short EDID
> data: manifacturer id
> [17514:17514:1020/165409:270374391297:ERROR:edid_parser.cc(116)] invalid EDID:
> human unreadable char in name
> [       OK ] EDIDParserTest.ParseBrokenEDID (0 ms)
> [ RUN      ] EDIDParserTest.GetDisplayId
> [       OK ] EDIDParserTest.GetDisplayId (0 ms)
> [ RUN      ] EDIDParserTest.GetDisplayIdFromInternal
> [       OK ] EDIDParserTest.GetDisplayIdFromInternal (0 ms)
> [ RUN      ] EDIDParserTest.GetDisplayIdFailure
> [17514:17514:1020/165409:270374391496:ERROR:edid_parser.cc(74)] too short EDID
> data: manifacturer id
> [       OK ] EDIDParserTest.GetDisplayIdFailure (0 ms)
> [----------] 7 tests from EDIDParserTest (0 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 9 tests from 2 test cases ran. (1 ms total)
> [  PASSED  ] 9 tests.
> 
> Don't seem to be calling display_configurator_unittest?

did you set chromeos=1 in $GYP_DEFINES?

[Daniel Erat, 2014-10-21 14:56:11]

On 2014/10/21 14:55:11, Daniel Erat wrote:
> On 2014/10/20 23:59:46, davidung wrote:
> > On 2014/10/20 22:38:40, Daniel Erat wrote:
> > > On 2014/10/20 22:33:47, davidung wrote:
> > > > On 2014/10/20 21:59:41, Daniel Erat wrote:
> > > > > build the appropriate test binary (display_unittests in this case) and
> > then
> > > > run
> > > > > it with --gtest_filter='DisplayConfiguratorTest.*'.
> > > > 
> > > > how do I build the display_unittests?
> > > > I am building the browser under the chromeos chroot with
> > > > 
> > > > $ CHROME_ORIGIN=LOCAL_SOURCE emerge-nyan chromeos-chrome
> > > > 
> > > > $ cd /var/cache/chromeos-chrome/chrome-src/src/out_nyan/Release
> > > > $ find ./ -name '*unittests'
> > > > ./sandbox_linux_unittests
> > > > ./media_unittests
> > > > 
> > > > is there a wiki I should be following?
> > > 
> > > oh, you shouldn't be using the chroot for this. do a regular non-chroot
> chrome
> > > build with chromeos=1 set via $GYP_DEFINES. see
> > > https://code.google.com/p/chromium/wiki/LinuxBuildInstructions if you
don't
> > know
> > > how to do this.
> > 
> > ok, thanks
> > after following http://code.google.com/p/chromium/wiki/LinuxChromiumArm  and
> > ninja -C out/Debug display_unittests
> > 
> > localhost tmp # ./display_unittests --single-process-tests
> > [17514:17514:1020/165409:270374390396:ERROR:memory_mapped_file.cc(41)]
> Couldn't
> > open /tmp/icudtl.dat
> > [17514:17514:1020/165409:270374390538:ERROR:icu_util.cc(149)] Couldn't mmap
> > /tmp/icudtl.dat
> > [==========] Running 9 tests from 2 test cases.
> > [----------] Global test environment set-up.
> > [----------] 2 tests from DisplayUtilTest
> > [ RUN      ] DisplayUtilTest.TestBlackListedDisplay
> > [       OK ] DisplayUtilTest.TestBlackListedDisplay (0 ms)
> > [ RUN      ] DisplayUtilTest.GetScaleFactor
> > [       OK ] DisplayUtilTest.GetScaleFactor (0 ms)
> > [----------] 2 tests from DisplayUtilTest (0 ms total)
> > 
> > [----------] 7 tests from EDIDParserTest
> > [ RUN      ] EDIDParserTest.ParseOverscanFlag
> > [       OK ] EDIDParserTest.ParseOverscanFlag (0 ms)
> > [ RUN      ] EDIDParserTest.ParseBrokenOverscanData
> > [       OK ] EDIDParserTest.ParseBrokenOverscanData (0 ms)
> > [ RUN      ] EDIDParserTest.ParseEDID
> > [       OK ] EDIDParserTest.ParseEDID (0 ms)
> > [ RUN      ] EDIDParserTest.ParseBrokenEDID
> > [17514:17514:1020/165409:270374391256:ERROR:edid_parser.cc(74)] too short
EDID
> > data: manifacturer id
> > [17514:17514:1020/165409:270374391297:ERROR:edid_parser.cc(116)] invalid
EDID:
> > human unreadable char in name
> > [       OK ] EDIDParserTest.ParseBrokenEDID (0 ms)
> > [ RUN      ] EDIDParserTest.GetDisplayId
> > [       OK ] EDIDParserTest.GetDisplayId (0 ms)
> > [ RUN      ] EDIDParserTest.GetDisplayIdFromInternal
> > [       OK ] EDIDParserTest.GetDisplayIdFromInternal (0 ms)
> > [ RUN      ] EDIDParserTest.GetDisplayIdFailure
> > [17514:17514:1020/165409:270374391496:ERROR:edid_parser.cc(74)] too short
EDID
> > data: manifacturer id
> > [       OK ] EDIDParserTest.GetDisplayIdFailure (0 ms)
> > [----------] 7 tests from EDIDParserTest (0 ms total)
> > 
> > [----------] Global test environment tear-down
> > [==========] 9 tests from 2 test cases ran. (1 ms total)
> > [  PASSED  ] 9 tests.
> > 
> > Don't seem to be calling display_configurator_unittest?
> 
> did you set chromeos=1 in $GYP_DEFINES?

(i suspect not, since it looks like your test binary doesn't include any files
from ui/display/chromeos. files with chromeos in their names get automatically
excluded if you're not doing a chrome os build.)

[davidung, 2014-10-21 21:54:37]

> > 
> > did you set chromeos=1 in $GYP_DEFINES?
> 
> (i suspect not, since it looks like your test binary doesn't include any files
> from ui/display/chromeos. files with chromeos in their names get automatically
> excluded if you're not doing a chrome os build.)

Yeah I figured that out, but after I did a git pull and gclient sync, its having
trouble cross-compiling

$ export GYP_DEFINES="chromeos=1 target_arch=arm arm_float_abi=hard"
$ export GYP_CROSSCOMPILE=1
$ build/gyp_chromium
$ ninja -C out/Debug display_unittests


<snip>

[10/1421] CXX
obj/third_party/skia/src/opts/skia_opts_neon.SkBitmapProcState_arm_neon.o
FAILED: ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF
obj/third_party/skia/src/opts/skia_opts_neon.SkBitmapProcState_arm_neon.o.d
-DV8_DEPRECATION_WARNINGS -D_FILE_OFFSET_BITS=64 -DCHROMIUM_BUILD
-DCR_CLANG_REVISION=218707 -DTOOLKIT_VIEWS=1 -DUI_COMPOSITOR_IMAGE_TRANSPORT
-DUSE_AURA=1 -DUSE_ASH=1 -DUSE_PANGO=1 -DUSE_CAIRO=1
-DUSE_DEFAULT_RENDER_THEME=1 -DUSE_X11=1 -DUSE_XI2_MT=2
-DIMAGE_LOADER_EXTENSION=1 -DENABLE_REMOTING=1 -DENABLE_WEBRTC=1
-DENABLE_PEPPER_CDMS -DENABLE_CONFIGURATION_POLICY -DENABLE_NOTIFICATIONS
-DENABLE_HIDPI=1 -DUSE_UDEV -DENABLE_EGLIMAGE=1 -DENABLE_TASK_MANAGER=1
-DENABLE_EXTENSIONS=1 -DENABLE_PLUGINS=1 -DENABLE_SESSION_SERVICE=1
-DENABLE_THEMES=1 -DENABLE_AUTOFILL_DIALOG=1 -DENABLE_BACKGROUND=1
-DENABLE_GOOGLE_NOW=1 -DCLD_VERSION=2 -DCLD2_DATA_SOURCE=static
-DENABLE_FULL_PRINTING=1 -DENABLE_PRINTING=1 -DENABLE_SPELLCHECK=1
-DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_APP_LIST=1 -DENABLE_MANAGED_USERS=1
-DENABLE_MDNS=1 -DENABLE_SERVICE_DISCOVERY=1 -DENABLE_LOAD_COMPLETION_HACKS=1
-DSK_ENABLE_INST_COUNT=0 -DSK_SUPPORT_GPU=1
'-DGR_GL_CUSTOM_SETUP_HEADER="GrGLConfig_chrome.h"'
-DSK_ENABLE_LEGACY_API_ALIASING=1 -DSK_ATTR_DEPRECATED=SK_NOTHING_ARG1
-DGR_GL_IGNORE_ES3_MSAA=0 -DSK_WILL_NEVER_DRAW_PERSPECTIVE_TEXT
-DSK_SUPPORT_LEGACY_TEXTRENDERMODE -DSK_LEGACY_NO_DISTANCE_FIELD_PATHS
-DSK_ALLOW_STATIC_GLOBAL_INITIALIZERS=0
-DSK_DISABLE_OFFSETIMAGEFILTER_OPTIMIZATION -DIGNORE_ROT_AA_RECT_OPT
-DSK_IGNORE_BLURRED_RRECT_OPT -DSK_GDI_ALWAYS_USE_TEXTMETRICS_FOR_FONT_METRICS
'-DSK_DEFAULT_FONT_CACHE_LIMIT=(20*1024*1024)'
-DSK_USE_DISCARDABLE_SCALEDIMAGECACHE -DSK_GAMMA_EXPONENT=1.2
-DSK_GAMMA_CONTRAST=0.2 -DSK_HIGH_QUALITY_IS_LANCZOS -DSK_USE_POSIX_THREADS
-DSK_ARM_HAS_NEON -DUSE_GLIB=1 -DUSE_NSS=1 -DOS_CHROMEOS=1
-DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -D_DEBUG
-D_GLIBCXX_DEBUG=1 -Igen -I../../third_party/skia/include/core
-I../../third_party/skia/include/effects -I../../third_party/skia/src/core
-I../../third_party/skia/src/opts -I../../third_party/skia/src/utils -I../..
-I../../skia/config -pthread -fno-strict-aliasing -Wno-unused-parameter
-Wno-missing-field-initializers -fvisibility=hidden -pipe -fPIC
-Wno-reserved-user-defined-literal -Xclang -load -Xclang
/home/davidu/chromium/src/third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so
-Xclang -add-plugin -Xclang find-bad-constructs -fcolor-diagnostics
-Wheader-hygiene -Wno-char-subscripts -Wno-unneeded-internal-declaration
-Wno-covered-switch-default -Wstring-conversion -Wno-c++11-narrowing
-Wno-deprecated-register -Wno-unused-local-typedef -mfpu=neon
-fomit-frame-pointer -Wno-format -Wno-unused-result -target arm-linux-gnueabihf
-march=armv7-a -no-integrated-as -mtune=generic-armv7-a -mfpu=neon
-mfloat-abi=hard -mthumb -O0 -g -gdwarf-4 -funwind-tables
-Wno-undefined-bool-conversion -Wno-tautological-undefined-compare
-I/usr/include/c++/4.6/arm-linux-gnueabihf -fno-exceptions -fno-rtti
-fno-threadsafe-statics -fvisibility-inlines-hidden -std=gnu++11 -Wno-deprecated
 -c ../../third_party/skia/src/opts/SkBitmapProcState_arm_neon.cpp -o
obj/third_party/skia/src/opts/skia_opts_neon.SkBitmapProcState_arm_neon.o
In file included from
../../third_party/skia/src/opts/SkBitmapProcState_arm_neon.cpp:8:
In file included from ../../third_party/skia/src/core/SkBitmapProcState.h:11:
In file included from ../../third_party/skia/include/core/SkBitmap.h:12:
In file included from ../../third_party/skia/include/core/SkColorTable.h:14:
In file included from ../../third_party/skia/include/core/SkFlattenable.h:11:
In file included from ../../third_party/skia/include/core/SkRefCnt.h:14:
In file included from ../../third_party/skia/include/core/SkThread.h:101:
In file included from ../../third_party/skia/src/ports/SkMutex_pthread.h:14:
/usr/include/pthread.h:665:6: error: 'regparm' is not valid on this platform
     __cleanup_fct_attribute;
     ^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/bits/pthreadtypes.h:222:50: note: expanded from macro
'__cleanup_fct_attribute'
# define __cleanup_fct_attribute __attribute__ ((__regparm__ (1)))
                                                 ^            ~
In file included from
../../third_party/skia/src/opts/SkBitmapProcState_arm_neon.cpp:8:
In file included from ../../third_party/skia/src/core/SkBitmapProcState.h:11:
In file included from ../../third_party/skia/include/core/SkBitmap.h:12:
In file included from ../../third_party/skia/include/core/SkColorTable.h:14:
In file included from ../../third_party/skia/include/core/SkFlattenable.h:11:
In file included from ../../third_party/skia/include/core/SkRefCnt.h:14:
In file included from ../../third_party/skia/include/core/SkThread.h:101:
In file included from ../../third_party/skia/src/ports/SkMutex_pthread.h:14:
/usr/include/pthread.h:677:3: error: 'regparm' is not valid on this platform
  __cleanup_fct_attribute;
  ^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/bits/pthreadtypes.h:222:50: note: expanded from macro
'__cleanup_fct_attribute'
# define __cleanup_fct_attribute __attribute__ ((__regparm__ (1)))
                                                 ^            ~
In file included from
../../third_party/skia/src/opts/SkBitmapProcState_arm_neon.cpp:8:
In file included from ../../third_party/skia/src/core/SkBitmapProcState.h:11:
In file included from ../../third_party/skia/include/core/SkBitmap.h:12:
In file included from ../../third_party/skia/include/core/SkColorTable.h:14:
In file included from ../../third_party/skia/include/core/SkFlattenable.h:11:
In file included from ../../third_party/skia/include/core/SkRefCnt.h:14:
In file included from ../../third_party/skia/include/core/SkThread.h:101:
In file included from ../../third_party/skia/src/ports/SkMutex_pthread.h:14:
/usr/include/pthread.h:700:6: error: 'regparm' is not valid on this platform
     __cleanup_fct_attribute;
     ^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/bits/pthreadtypes.h:222:50: note: expanded from macro
'__cleanup_fct_attribute'
# define __cleanup_fct_attribute __attribute__ ((__regparm__ (1)))
                                                 ^            ~
In file included from
../../third_party/skia/src/opts/SkBitmapProcState_arm_neon.cpp:8:
In file included from ../../third_party/skia/src/core/SkBitmapProcState.h:11:
In file included from ../../third_party/skia/include/core/SkBitmap.h:12:
In file included from ../../third_party/skia/include/core/SkColorTable.h:14:
In file included from ../../third_party/skia/include/core/SkFlattenable.h:11:
In file included from ../../third_party/skia/include/core/SkRefCnt.h:14:
In file included from ../../third_party/skia/include/core/SkThread.h:101:
In file included from ../../third_party/skia/src/ports/SkMutex_pthread.h:14:
/usr/include/pthread.h:713:3: error: 'regparm' is not valid on this platform
  __cleanup_fct_attribute;
  ^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/bits/pthreadtypes.h:222:50: note: expanded from macro
'__cleanup_fct_attribute'
# define __cleanup_fct_attribute __attribute__ ((__regparm__ (1)))
                                                 ^            ~
In file included from
../../third_party/skia/src/opts/SkBitmapProcState_arm_neon.cpp:8:
In file included from ../../third_party/skia/src/core/SkBitmapProcState.h:11:
In file included from ../../third_party/skia/include/core/SkBitmap.h:12:
In file included from ../../third_party/skia/include/core/SkColorTable.h:14:
In file included from ../../third_party/skia/include/core/SkFlattenable.h:11:
In file included from ../../third_party/skia/include/core/SkRefCnt.h:14:
In file included from ../../third_party/skia/include/core/SkThread.h:101:
In file included from ../../third_party/skia/src/ports/SkMutex_pthread.h:14:
/usr/include/pthread.h:718:6: error: 'regparm' is not valid on this platform
     __cleanup_fct_attribute __attribute__ ((__noreturn__))
     ^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/bits/pthreadtypes.h:222:50: note: expanded from macro
'__cleanup_fct_attribute'
# define __cleanup_fct_attribute __attribute__ ((__regparm__ (1)))
                                                 ^            ~
In file included from
../../third_party/skia/src/opts/SkBitmapProcState_arm_neon.cpp:8:
In file included from ../../third_party/skia/src/core/SkBitmapProcState.h:11:
In file included from ../../third_party/skia/include/core/SkBitmap.h:12:
In file included from ../../third_party/skia/include/core/SkColorTable.h:14:
In file included from ../../third_party/skia/include/core/SkFlattenable.h:11:
In file included from ../../third_party/skia/include/core/SkRefCnt.h:16:
In file included from ../../third_party/skia/include/core/SkTemplates.h:15:
/usr/include/c++/4.6/new:41:10: fatal error: 'bits/c++config.h' file not found
#include <bits/c++config.h>
         ^
6 errors generated.
ninja: build stopped: subcommand failed.

[Daniel Erat, 2014-10-21 21:59:06]

sorry, i probably can't be any more help -- i've never done an arm build outside
of the chroot.

[davidung, 2014-10-21 22:08:12]

On 2014/10/21 21:59:06, Daniel Erat wrote:
> sorry, i probably can't be any more help -- i've never done an arm build
outside
> of the chroot.

thx, can you add the right person that know how to cross-compile for arm?

[Daniel Erat, 2014-10-21 22:11:37]

On 2014/10/21 22:08:12, davidung wrote:
> On 2014/10/21 21:59:06, Daniel Erat wrote:
> > sorry, i probably can't be any more help -- i've never done an arm build
> outside
> > of the chroot.
> 
> thx, can you add the right person that know how to cross-compile for arm?

not sure, but you could try asking on the chromium-os-dev or chromium-dev
mailing lists.

[Daniel Erat, 2014-10-21 22:12:38]

On 2014/10/21 22:11:37, Daniel Erat wrote:
> On 2014/10/21 22:08:12, davidung wrote:
> > On 2014/10/21 21:59:06, Daniel Erat wrote:
> > > sorry, i probably can't be any more help -- i've never done an arm build
> > outside
> > > of the chroot.
> > 
> > thx, can you add the right person that know how to cross-compile for arm?
> 
> not sure, but you could try asking on the chromium-os-dev or chromium-dev
> mailing lists.

side question: why are you trying to build these tests for arm? if you're
building on intel, you should be able to just run the unit tests there.

[davidung, 2014-10-21 23:11:40]

On 2014/10/21 22:12:38, Daniel Erat wrote:
> On 2014/10/21 22:11:37, Daniel Erat wrote:
> > On 2014/10/21 22:08:12, davidung wrote:
> > > On 2014/10/21 21:59:06, Daniel Erat wrote:
> > > > sorry, i probably can't be any more help -- i've never done an arm build
> > > outside
> > > > of the chroot.
> > > 
> > > thx, can you add the right person that know how to cross-compile for arm?
> > 
> > not sure, but you could try asking on the chromium-os-dev or chromium-dev
> > mailing lists.
> 
> side question: why are you trying to build these tests for arm? if you're
> building on intel, you should be able to just run the unit tests there.

well this patch originates from a bug that's for ARM platform, so I would think
one should at least test it there.
sure I can probably build and test it on an x86 chromebook

[Daniel Erat, 2014-10-21 23:26:43]

On 2014/10/21 23:11:40, davidung wrote:
> On 2014/10/21 22:12:38, Daniel Erat wrote:
> > On 2014/10/21 22:11:37, Daniel Erat wrote:
> > > On 2014/10/21 22:08:12, davidung wrote:
> > > > On 2014/10/21 21:59:06, Daniel Erat wrote:
> > > > > sorry, i probably can't be any more help -- i've never done an arm
build
> > > > outside
> > > > > of the chroot.
> > > > 
> > > > thx, can you add the right person that know how to cross-compile for
arm?
> > > 
> > > not sure, but you could try asking on the chromium-os-dev or chromium-dev
> > > mailing lists.
> > 
> > side question: why are you trying to build these tests for arm? if you're
> > building on intel, you should be able to just run the unit tests there.
> 
> well this patch originates from a bug that's for ARM platform, so I would
think
> one should at least test it there.
> sure I can probably build and test it on an x86 chromebook

you should manually test your fix using an arm build of chrome os, but the unit
test should be platform-agnostic. there's no reason to run the test on a
chromebook; just do a regular x86 build on your workstation with chromeos=1 and
run the test there.

[davidung, 2014-10-22 01:39:06]

Patset 2 uploaded with unittest added.

[1/37] DisplayConfiguratorTest.PanelFitting (0 ms)
[2/37] DisplayConfiguratorTest.ContentProtection (1 ms)
[3/37] DisplayConfiguratorTest.ContentProtectionTwoClients (0 ms)
[4/37] DisplayConfiguratorTest.ContentProtectionTwoClientsEnable (0 ms)
[5/37] DisplayConfiguratorTest.HandleConfigureCrtcFailure (0 ms)
[6/37] DisplayConfiguratorTest.SaveDisplayPowerStateOnConfigFailure (0 ms)
[7/37] DisplayConfiguratorTest.DontRestoreStalePowerStateAfterResume (0 ms)
[8/37] DisplayUtilX11Test.GetDisplayConnectionTypeFromName (1 ms)
[9/37] NativeDisplayEventDispatcherX11Test.OnScreenChangedEvent (0 ms)
[10/37] NativeDisplayEventDispatcherX11Test.CheckNotificationOnFirstEvent (0 ms)
[11/37] NativeDisplayEventDispatcherX11Test.CheckNotificationAfterSecondEvent (0
ms)
[12/37] NativeDisplayEventDispatcherX11Test.CheckNotificationOnDisconnect (0 ms)
[13/37] NativeDisplayEventDispatcherX11Test.CheckNotificationOnModeChange (0 ms)
[14/37] NativeDisplayEventDispatcherX11Test.CheckNotificationOnSecondOutput (0
ms)
[15/37] NativeDisplayEventDispatcherX11Test.CheckNotificationOnDifferentCrtc (0
ms)
[16/37]
NativeDisplayEventDispatcherX11Test.CheckNotificationOnSecondOutputDisconnect (0
ms)
[17/37]
NativeDisplayEventDispatcherX11Test.AvoidDuplicateNotificationOnSecondOutputDisconnect
(0 ms)
[18/37] NativeDisplayEventDispatcherX11Test.ForceUpdateAfterCacheExpiration (0
ms)
[19/37] DisplayUtilTest.TestBlackListedDisplay (0 ms)
[20/37] DisplayUtilTest.GetScaleFactor (0 ms)
[21/37] EDIDParserTest.ParseOverscanFlag (0 ms)
[22/37] EDIDParserTest.ParseBrokenOverscanData (0 ms)
[23/37] EDIDParserTest.ParseEDID (0 ms)
[24/37] EDIDParserTest.ParseBrokenEDID (0 ms)
[25/37] EDIDParserTest.GetDisplayId (0 ms)
[26/37] EDIDParserTest.GetDisplayIdFromInternal (0 ms)
[27/37] EDIDParserTest.GetDisplayIdFailure (0 ms)
[28/37] DisplayConfiguratorTest.FindDisplayModeMatchingSize (0 ms)
[29/37] DisplayConfiguratorTest.ConnectSecondOutput (0 ms)
[30/37] DisplayConfiguratorTest.SetDisplayPower (0 ms)
[31/37] DisplayConfiguratorTest.SuspendAndResume (0 ms)
[32/37] DisplayConfiguratorTest.Headless (0 ms)
[33/37] DisplayConfiguratorTest.StartWithTwoOutputs (0 ms)
[34/37] DisplayConfiguratorTest.InvalidMultipleDisplayStates (1 ms)
[35/37] DisplayConfiguratorTest.GetMultipleDisplayStateForDisplaysWithoutId (0
ms)
[36/37] DisplayConfiguratorTest.GetMultipleDisplayStateForDisplaysWithId (0 ms)
[37/37] DisplayConfiguratorTest.UpdateCachedOutputsEvenAfterFailure (0 ms)
SUCCESS: all tests passed.
Tests took 0 seconds.

[Daniel Erat, 2014-10-22 17:04:03]

lgtm with one comment

https://codereview.chromium.org/606913002/diff/50001/ui/display/chromeos/disp...
File ui/display/chromeos/display_configurator_unittest.cc (right):

https://codereview.chromium.org/606913002/diff/50001/ui/display/chromeos/disp...
ui/display/chromeos/display_configurator_unittest.cc:439: DisplayMode
*native_mode;
nit: please initialize this when it's declared, e.g.

  DisplayMode* native mode =
      new DisplayMode(gfx::Size(1920, 1200), false, 50.0);
  modes.push_back(native_mode);

(also note that the '*' goes to the left of the space)

[davidung, 2014-10-23 22:00:52]

The CQ bit was checked by davidu@nvidia.com

[commit-bot: I haz the power, 2014-10-23 22:03:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/606913002/70001

[commit-bot: I haz the power, 2014-10-23 23:26:24]

Committed patchset #3 (id:70001)

[commit-bot: I haz the power, 2014-10-23 23:26:44]

Patchset 3 (id:??) landed as
https://crrev.com/6923c2bc95e85e4ef800101a534744c03b25048a
Cr-Commit-Position: refs/heads/master@{#300976}