Bug 79628 – Add a ChromiumGpuAndroid port.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Hao Zheng]

Revert https://bugs.webkit.org/show_bug.cgi?id=78580 , as we do have a ChromiumGpuAndroid port. Will send out a patch.

[Adam Barth]

We're in the process of deleting all the GPU ports, but it's probably fine to add this for the time being.

[Hao Zheng]

Created attachment 128962 [details]
Patch

[Hao Zheng]

(In reply to comment #1)
> We're in the process of deleting all the GPU ports, but it's probably fine to add this for the time being.

I see. Actually, I found our gpu port broken in our merge, mostly due to the the process of removing all gpu ports (code is undergoing lots of change). So I want to upstream this first, hoping that future refactoring could take Android port into account, too. Thanks for upstreaming our Android port, Adam.

[Adam Barth]

Comment on attachment 128962 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=128962&action=review

> Tools/Scripts/webkitpy/layout_tests/port/factory.py:76
> -            return 'chromium-linux'
> +            if os.environ.get('GYP_DEFINES', '').find('OS=android') >= 0:
> +                return 'chromium-android'
> +            else:
> +                return 'chromium-linux'

This part of the change is wrong.  If you want to create the the chromium-android port, you need to pass it on the command line:

--platform=chromium-android

> Tools/Scripts/webkitpy/layout_tests/port/factory.py:100
> -            port_name = port_name + '-' + self._host.platform.os_name
> +            if os.environ.get('GYP_DEFINES', '').find('OS=android') >= 0:
> +                port_name = 'chromium-gpu-android'
> +            else:
> +                port_name = port_name + '-' + self._host.platform.os_name

Same here.

[Adam Barth]

(Other than that, looks good.)

[Hao Zheng]

Created attachment 128965 [details]
Patch

[Hao Zheng]

(In reply to comment #4)
> (From update of attachment 128962 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=128962&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/port/factory.py:76
> > -            return 'chromium-linux'
> > +            if os.environ.get('GYP_DEFINES', '').find('OS=android') >= 0:
> > +                return 'chromium-android'
> > +            else:
> > +                return 'chromium-linux'
> 
> This part of the change is wrong.  If you want to create the the chromium-android port, you need to pass it on the command line:
> 
> --platform=chromium-android
> 

I see. Done.

> > Tools/Scripts/webkitpy/layout_tests/port/factory.py:100
> > -            port_name = port_name + '-' + self._host.platform.os_name
> > +            if os.environ.get('GYP_DEFINES', '').find('OS=android') >= 0:
> > +                port_name = 'chromium-gpu-android'
> > +            else:
> > +                port_name = port_name + '-' + self._host.platform.os_name
> 
> Same here.

Done.

[Adam Barth]

Comment on attachment 128965 [details]
Patch

Thanks!

[Hao Zheng]

(In reply to comment #8)
> (From update of attachment 128965 [details])
> Thanks!

Thanks for your really quick review!

[WebKit Review Bot]

Comment on attachment 128965 [details]
Patch

Clearing flags on attachment: 128965

Committed r108974: <http://trac.webkit.org/changeset/108974>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dirk Pranke]

I'm confused; I thought James said that that the gpu path is always used on the Android port, so why is this needed?

[James Robinson]

Yeah I'm quite confused by this as well.  You need a GPU version of a port when you want to run tests in a GPU and non-GPU configuration and maintain separate baselines and possibly flags for the two.  Is this really true for the Android port?

[Hao Zheng]

(In reply to comment #13)
> Yeah I'm quite confused by this as well.  You need a GPU version of a port when you want to run tests in a GPU and non-GPU configuration and maintain separate baselines and possibly flags for the two.  Is this really true for the Android port?

Sorry for confusion. Yes, we only has hardware gpu path in browser. However, our hardware gpu path is still not reliable enough for running layout tests, as it crashes randomly sometimes (not very often, but does crash flakily). Maybe it's because of some memory leak issues. So on our downstream bot, we actually run non gpu tests for now. That's why we need a non gpu port and a standalone gpu port now. When the hardware gpu path is reliable enough to run tens of thousands of tests, we can run layout tests completely in hardware gpu path and remove the gpu port.

[James Robinson]

So you aren't testing the configuration you are shipping?  That seems....brave

[Hao Zheng]

(In reply to comment #15)
> So you aren't testing the configuration you are shipping?  That seems....brave

I keep an eye on them locally. Of course that's bad... I hope we can fix it very soon.