Adds a do-nothing ozone platform

Changes test ozone platform to not produce any output when --ozone-dump-file is not specified. Also modifies the test ozone platform to not implement any event dispatch.

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

[varkha, 2014-07-25 23:11:23]

Can you please take a look? This is so we don't have to use test ozone platform
on a system with no visuals.

[spang, 2014-07-28 19:33:40]

On 2014/07/25 23:11:23, varkha wrote:
> Can you please take a look? This is so we don't have to use test ozone
platform
> on a system with no visuals.

How about:

1. Remove evdev from the test platform. The test platform runs on the buildbots
and shouldn't try to open devices.

2. Modify the "test" platform to be the null platform, when there's no
--ozone-dump-file specified.

[spang, 2014-07-30 21:13:05]

On 2014/07/28 19:33:40, spang wrote:
> On 2014/07/25 23:11:23, varkha wrote:
> > Can you please take a look? This is so we don't have to use test ozone
> platform
> > on a system with no visuals.
> 
> How about:
> 
> 1. Remove evdev from the test platform. The test platform runs on the
buildbots
> and shouldn't try to open devices.
> 

Danny did this part today

> 2. Modify the "test" platform to be the null platform, when there's no
> --ozone-dump-file specified.

[varkha, 2014-07-31 01:19:09]

Thanks for heads up. PTAL.

[spang, 2014-07-31 02:19:35]

lgtm

[alexst (slow to review), 2014-07-31 02:27:24]

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/o...
File ui/ozone/platform/test/ozone_platform_test.cc (right):

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/o...
ui/ozone/platform/test/ozone_platform_test.cc:75: if (file_path_.empty())
Is there a particular reason you chose to skip the factories below?

I am not convinced this is safe to do because call-sites elsewhere in chrome
assume factories are not null.

[dnicoara, 2014-07-31 02:51:31]

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/o...
File ui/ozone/platform/test/ozone_platform_test.cc (right):

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/o...
ui/ozone/platform/test/ozone_platform_test.cc:75: if (file_path_.empty())
On 2014/07/31 02:27:24, alexst wrote:
> Is there a particular reason you chose to skip the factories below?
> 
> I am not convinced this is safe to do because call-sites elsewhere in chrome
> assume factories are not null.

From personal experience, the following factories will be required.

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
File ui/ozone/platform/test/test_window.cc (left):

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
ui/ozone/platform/test/test_window.cc:22:
ui::PlatformEventSource::GetInstance()->AddPlatformEventDispatcher(this);
If this isn't needed then I think you can remove the PlatformEventDispatcher
inheritance and the associated overrides.

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
File ui/ozone/platform/test/test_window_manager.cc (right):

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
ui/ozone/platform/test/test_window_manager.cc:105: if (location_.empty()) {
There is no reason to complicate the code with the if statements (modulo the one
in Initialize) since the end result is the same.

[spang, 2014-07-31 16:04:01]

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
File ui/ozone/platform/test/test_window_manager.cc (right):

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
ui/ozone/platform/test/test_window_manager.cc:105: if (location_.empty()) {
On 2014/07/31 02:51:31, dnicoara wrote:
> There is no reason to complicate the code with the if statements (modulo the
one
> in Initialize) since the end result is the same.

I think we need the one in Initialize() and in PresentCanvas(). The rest are
probably not needed.

[dnicoara, 2014-07-31 16:09:23]

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
File ui/ozone/platform/test/test_window_manager.cc (right):

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
ui/ozone/platform/test/test_window_manager.cc:105: if (location_.empty()) {
On 2014/07/31 16:04:01, spang wrote:
> On 2014/07/31 02:51:31, dnicoara wrote:
> > There is no reason to complicate the code with the if statements (modulo the
> one
> > in Initialize) since the end result is the same.
> 
> I think we need the one in Initialize() and in PresentCanvas(). The rest are
> probably not needed.

The one in CreateCanvasForWidget shouldn't be needed, window->path() returns an
empty path as well.

[varkha, 2014-08-07 00:17:11]

Getting back to this. PTAL.

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/o...
File ui/ozone/platform/test/ozone_platform_test.cc (right):

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/o...
ui/ozone/platform/test/ozone_platform_test.cc:75: if (file_path_.empty())
On 2014/07/31 02:27:24, alexst wrote:
> Is there a particular reason you chose to skip the factories below?
> 
> I am not convinced this is safe to do because call-sites elsewhere in chrome
> assume factories are not null.

Acknowledged.

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/o...
ui/ozone/platform/test/ozone_platform_test.cc:75: if (file_path_.empty())
On 2014/07/31 02:51:31, dnicoara wrote:
> On 2014/07/31 02:27:24, alexst wrote:
> > Is there a particular reason you chose to skip the factories below?
> > 
> > I am not convinced this is safe to do because call-sites elsewhere in chrome
> > assume factories are not null.
> 
> From personal experience, the following factories will be required.

Done.

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
File ui/ozone/platform/test/test_window.cc (left):

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
ui/ozone/platform/test/test_window.cc:22:
ui::PlatformEventSource::GetInstance()->AddPlatformEventDispatcher(this);
On 2014/07/31 02:51:31, dnicoara wrote:
> If this isn't needed then I think you can remove the PlatformEventDispatcher
> inheritance and the associated overrides.

Done.

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
File ui/ozone/platform/test/test_window_manager.cc (right):

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
ui/ozone/platform/test/test_window_manager.cc:105: if (location_.empty()) {
On 2014/07/31 02:51:31, dnicoara wrote:
> There is no reason to complicate the code with the if statements (modulo the
one
> in Initialize) since the end result is the same.

Acknowledged.

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
ui/ozone/platform/test/test_window_manager.cc:105: if (location_.empty()) {
On 2014/07/31 16:04:01, spang wrote:
> On 2014/07/31 02:51:31, dnicoara wrote:
> > There is no reason to complicate the code with the if statements (modulo the
> one
> > in Initialize) since the end result is the same.
> 
> I think we need the one in Initialize() and in PresentCanvas(). The rest are
> probably not needed.

Done.

https://codereview.chromium.org/419993003/diff/20001/ui/ozone/platform/test/t...
ui/ozone/platform/test/test_window_manager.cc:105: if (location_.empty()) {
On 2014/07/31 16:09:23, dnicoara wrote:
> On 2014/07/31 16:04:01, spang wrote:
> > On 2014/07/31 02:51:31, dnicoara wrote:
> > > There is no reason to complicate the code with the if statements (modulo
the
> > one
> > > in Initialize) since the end result is the same.
> > 
> > I think we need the one in Initialize() and in PresentCanvas(). The rest are
> > probably not needed.
> 
> The one in CreateCanvasForWidget shouldn't be needed, window->path() returns
an
> empty path as well.

Done.

[dnicoara, 2014-08-07 00:32:40]

lgtm

Would you mind waiting until tomorrow when ash_unittests are enabled? Then
please run the Ozone trybots to see if anything fails.

[varkha, 2014-08-07 00:54:37]

On 2014/08/07 00:32:40, dnicoara wrote:
> lgtm
> 
> Would you mind waiting until tomorrow when ash_unittests are enabled? Then
> please run the Ozone trybots to see if anything fails.

np. Will wait. Thanks!

[dnicoara, 2014-08-07 15:04:38]

Forgot to mention, maybe the CL title should be updated to better reflect the
changes.

[varkha, 2014-08-07 16:54:44]

On 2014/08/07 15:04:38, dnicoara wrote:
> Forgot to mention, maybe the CL title should be updated to better reflect the
> changes.

Done.

[varkha, 2014-08-07 19:06:02]

The CQ bit was checked by varkha@chromium.org

[varkha, 2014-08-07 19:08:15]

interactive_ui_tests still has a few failures on
linux_chromium_chromeos_ozone_rel. I assume it is unrelated.

[commit-bot: I haz the power, 2014-08-07 19:12:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/varkha@chromium.org/419993003/60001

[commit-bot: I haz the power, 2014-08-07 20:12:36]

Change committed as 288132