Revert https://bugs.webkit.org/show_bug.cgi?id=78580 , as we do have a ChromiumGpuAndroid port. Will send out a patch.
We're in the process of deleting all the GPU ports, but it's probably fine to add this for the time being.
Created attachment 128962 [details] Patch
(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.
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.
(Other than that, looks good.)
Created attachment 128965 [details] Patch
(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.
Comment on attachment 128965 [details] Patch Thanks!
(In reply to comment #8) > (From update of attachment 128965 [details]) > Thanks! Thanks for your really quick review!
Comment on attachment 128965 [details] Patch Clearing flags on attachment: 128965 Committed r108974: <http://trac.webkit.org/changeset/108974>
All reviewed patches have been landed. Closing bug.
I'm confused; I thought James said that that the gpu path is always used on the Android port, so why is this needed?
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?
(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.
So you aren't testing the configuration you are shipping? That seems....brave
(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.