[Chromedriver] Add Device Metrics override support to ChromeDriver via Capabilities

[Chromedriver] Add Device Metrics override support to ChromeDriver via Capabilities

BUG=chromedriver:399
All of the code reviews for this patch were written against https://codereview.chromium.org/251933005/ . I used the wrong email address to create said patch, so I have created this one with the correct email address.

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

[srawlins, 2014-05-20 18:47:02]

The CQ bit was checked by srawlins@google.com

[commit-bot: I haz the power, 2014-05-20 18:48:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/srawlins@google.com/288193004/1

[commit-bot: I haz the power, 2014-05-20 18:48:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-20 18:48:25]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[samuong, 2014-05-20 18:52:16]

On 2014/05/20 18:48:25, I haz the power (commit-bot) wrote:
> No LGTM from a valid reviewer yet. Only full committers are accepted.
> Even if an LGTM may have been provided, it was from a non-committer or
> a provisional committer, _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.

lgtm

[pfeldman, 2014-05-20 18:57:28]

https://codereview.chromium.org/288193004/diff/1/chrome/test/chromedriver/chr...
File chrome/test/chromedriver/chrome/mobile_emulation_override_manager.cc
(right):

https://codereview.chromium.org/288193004/diff/1/chrome/test/chromedriver/chr...
chrome/test/chromedriver/chrome/mobile_emulation_override_manager.cc:47:
params.SetBoolean("emulateViewport",
Note that you are using undocumented API that is a subject to change.

@dgozman: you were planning to rename this to 'mobile'. Are you sure you want to
do this? If yes, we could help via making it sooner.

[dgozman, 2014-05-20 19:01:39]

https://codereview.chromium.org/288193004/diff/1/chrome/test/chromedriver/chr...
File chrome/test/chromedriver/chrome/mobile_emulation_override_manager.cc
(right):

https://codereview.chromium.org/288193004/diff/1/chrome/test/chromedriver/chr...
chrome/test/chromedriver/chrome/mobile_emulation_override_manager.cc:47:
params.SetBoolean("emulateViewport",
On 2014/05/20 18:57:29, pfeldman wrote:
> Note that you are using undocumented API that is a subject to change.
> 
> @dgozman: you were planning to rename this to 'mobile'. Are you sure you want
to
> do this? If yes, we could help via making it sooner.

Yes, I want to do this to emphasize it's not only the viewport emulation, but
also some other mobile-only aspects like screen position or compositor tweaks in
the future. Feel free to do it sooner.

[stgao, 2014-05-20 19:28:06]

lgtm

[srawlins, 2014-05-20 19:36:15]

https://codereview.chromium.org/288193004/diff/1/chrome/test/chromedriver/chr...
File chrome/test/chromedriver/chrome/mobile_emulation_override_manager.cc
(right):

https://codereview.chromium.org/288193004/diff/1/chrome/test/chromedriver/chr...
chrome/test/chromedriver/chrome/mobile_emulation_override_manager.cc:47:
params.SetBoolean("emulateViewport",
On 2014/05/20 19:01:40, dgozman wrote:
> On 2014/05/20 18:57:29, pfeldman wrote:
> > Note that you are using undocumented API that is a subject to change.
> > 
> > @dgozman: you were planning to rename this to 'mobile'. Are you sure you
want
> to
> > do this? If yes, we could help via making it sooner.
> 
> Yes, I want to do this to emphasize it's not only the viewport emulation, but
> also some other mobile-only aspects like screen position or compositor tweaks
in
> the future. Feel free to do it sooner.

@pfeldman, @dgozman: SGTM. We can update chromedriver to accept old and new name
once that change is written and part of chrome.

[srawlins, 2014-05-20 19:36:21]

The CQ bit was checked by srawlins@google.com

[commit-bot: I haz the power, 2014-05-20 19:37:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/srawlins@google.com/288193004/1

[commit-bot: I haz the power, 2014-05-21 07:45:23]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)

[stgao, 2014-05-21 17:23:07]

The CQ bit was unchecked by stgao@chromium.org

[stgao, 2014-05-21 17:23:13]

The CQ bit was checked by stgao@chromium.org

[commit-bot: I haz the power, 2014-05-21 19:57:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/srawlins@google.com/288193004/1

[commit-bot: I haz the power, 2014-05-21 20:04:51]

Change committed as 271947

[stgao, 2014-05-22 05:02:22]

https://codereview.chromium.org/288193004/diff/1/chrome/test/chromedriver/tes...
File chrome/test/chromedriver/test/run_py_tests.py (right):

https://codereview.chromium.org/288193004/diff/1/chrome/test/chromedriver/tes...
chrome/test/chromedriver/test/run_py_tests.py:852: def testDeviceName(self):
It seems this testcase keeps failing on Chrome 33, but passes on Chrome 34, 35,
and trunk.
Although we are going to deprecate support on Chrome 33 as 35 is promoted to
stable, it is still good to know why it fails.

http://build.chromium.org/p/chromium.chromedriver/builders/Linux/builds/6016/...

http://build.chromium.org/p/chromium.chromedriver/waterfall

Would you mind taking a look?
Chrome 33 could be downloaded from
https://commondatastorage.googleapis.com/chromium-browser-continuous/index.ht...

[samuong, 2014-05-22 17:13:19]

On 2014/05/22 05:02:22, Shuotao wrote:
>
https://codereview.chromium.org/288193004/diff/1/chrome/test/chromedriver/tes...
> File chrome/test/chromedriver/test/run_py_tests.py (right):
> 
>
https://codereview.chromium.org/288193004/diff/1/chrome/test/chromedriver/tes...
> chrome/test/chromedriver/test/run_py_tests.py:852: def testDeviceName(self):
> It seems this testcase keeps failing on Chrome 33, but passes on Chrome 34,
35,
> and trunk.
> Although we are going to deprecate support on Chrome 33 as 35 is promoted to
> stable, it is still good to know why it fails.
> 
>
http://build.chromium.org/p/chromium.chromedriver/builders/Linux/builds/6016/...
> 
> http://build.chromium.org/p/chromium.chromedriver/waterfall
> 
> Would you mind taking a look?
> Chrome 33 could be downloaded from
>
https://commondatastorage.googleapis.com/chromium-browser-continuous/index.ht...

Here's the error that we're getting:

UnknownError: unknown error: unhandled inspector error:
{"code":-32000,"message":"Compositing mode is not supported"}

My knee-jerk reaction was to blame Xvfb but we're also seeing this on Mac, also
only on M33, and also only for this mobile emulation test.

[chromium-reviews, 2014-05-22 17:49:43]

Wow, a curious error indeed! I can reproduce locally, on x86-64 Linux:

chrome head, no Xvfb ==> OK
chrome 33; no Xvfb ==> OK
chrome head, yes Xvfb ==> OK
chrome 33; yes Xvfb ==> ERROR

Based on
this<http://stackoverflow.com/questions/23269807/devtools-compositing-mode-is-not-supported>,
it may be an "Accelerated Compositing" issue in Chrome 33? Maybe fixed
by dgozman in 50043007 <https://codereview.chromium.org/50043007> ("DevTools:
check for canForceCompositingMode before trying to emulate device
metrics.") or maybe by caseq in
27330002<https://codereview.chromium.org/27330002>("DevTools: disable
"force compositing" setting unless both modes are
supported"). Not sure...


Any recommendations on how to move forward? I'll keep digging.




On Thu, May 22, 2014 at 10:13 AM, <samuong@chromium.org> wrote:

> On 2014/05/22 05:02:22, Shuotao wrote:
>
> https://codereview.chromium.org/288193004/diff/1/chrome/
> test/chromedriver/test/run_py_tests.py
>
>> File chrome/test/chromedriver/test/run_py_tests.py (right):
>>
>
>
> https://codereview.chromium.org/288193004/diff/1/chrome/
> test/chromedriver/test/run_py_tests.py#newcode852
>
>> chrome/test/chromedriver/test/run_py_tests.py:852: def
>> testDeviceName(self):
>> It seems this testcase keeps failing on Chrome 33, but passes on Chrome
>> 34,
>>
> 35,
>
>> and trunk.
>> Although we are going to deprecate support on Chrome 33 as 35 is promoted
>> to
>> stable, it is still good to know why it fails.
>>
>
>
> http://build.chromium.org/p/chromium.chromedriver/
> builders/Linux/builds/6016/steps/python_tests%28v33%29/logs/stdio
>
>  http://build.chromium.org/p/chromium.chromedriver/waterfall
>>
>
>  Would you mind taking a look?
>> Chrome 33 could be downloaded from
>>
>
> https://commondatastorage.googleapis.com/chromium-
> browser-continuous/index.html?path=Linux_x64/241075/
>
> Here's the error that we're getting:
>
> UnknownError: unknown error: unhandled inspector error:
> {"code":-32000,"message":"Compositing mode is not supported"}
>
> My knee-jerk reaction was to blame Xvfb but we're also seeing this on Mac,
> also
> only on M33, and also only for this mobile emulation test.
>
> https://codereview.chromium.org/288193004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[samuong, 2014-05-22 19:03:59]

On 2014/05/22 17:49:43, chromium-reviews_chromium.org wrote:
> Wow, a curious error indeed! I can reproduce locally, on x86-64 Linux:
> 
> chrome head, no Xvfb ==> OK
> chrome 33; no Xvfb ==> OK
> chrome head, yes Xvfb ==> OK
> chrome 33; yes Xvfb ==> ERROR
> 
> Based on
>
this<http://stackoverflow.com/questions/23269807/devtools-compositing-mode-is-not-supported>,
> it may be an "Accelerated Compositing" issue in Chrome 33? Maybe fixed
> by dgozman in 50043007 <https://codereview.chromium.org/50043007> ("DevTools:
> check for canForceCompositingMode before trying to emulate device
> metrics.") or maybe by caseq in
> 27330002<https://codereview.chromium.org/27330002>("DevTools: disable
> "force compositing" setting unless both modes are
> supported"). Not sure...
> 
> 
> Any recommendations on how to move forward? I'll keep digging.

We now know that it is a compositing issue that was fixed in M34+. We could run
a bisect to figure out precisely what CL fixed it but I don't think this would
be a very good use of your time. As Shuotao mentioned, it's time to drop support
for M33 anyway, since we just pushed out M35 to stable a few days ago. So I'll
submit a CL to drop support for M33 today, and you don't have to worry about
this particular issue.