82033 2012-03-23 01:31:22 -0700 Make chromium android port use hardware gpu path default. 2012-04-26 11:56:58 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 zhenghao zhenghao abarth dpranke ojan peter webkit.review.bot oldest_to_newest 586166 0 zhenghao 2012-03-23 01:31:22 -0700 Make chromium android port use hardware gpu path default. 586167 1 133443 zhenghao 2012-03-23 01:33:00 -0700 Created attachment 133443 Patch 586310 2 133443 peter 2012-03-23 07:14:23 -0700 Comment on attachment 133443 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133443&action=review Thanks! Some drive-by nits. > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:1 > +#!/usr/bin/env python Do we invoke chromium_android.py directly? I don't think this is necessary if it's just being imported elsewhere. > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:137 > + 'chromium-gpu', These names correspond to directories in LayoutTests/platform/. All the chromium-gpu-* directories got removed in r110517. > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:148 > + # The chromium-android port always uses the hardware GPU code path, textual nit: "The Chromium port for Android always uses the hardware GPU path." > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:237 > + # All tests run in hardware gpu path, so don't need to run virtual gpu tests. textual nit: "All tests are being run in the hardware GPU path, so there is no need to run virtual GPU tests." 586386 3 133443 abarth 2012-03-23 09:15:15 -0700 Comment on attachment 133443 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133443&action=review > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:149 > + self._graphics_type = 'gpu' Does graphics_type still exist? It seems like it should be removed now that the GPU configuration no longer exists. 586492 4 133443 dpranke 2012-03-23 11:00:51 -0700 Comment on attachment 133443 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133443&action=review >> Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:149 >> + self._graphics_type = 'gpu' > > Does graphics_type still exist? It seems like it should be removed now that the GPU configuration no longer exists. No, it doesn't still exist. > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:238 > + return [] To be more future-proof, you might want to call the base class and then remove the gpu suites, in case other suites get added down the road. 587431 5 zhenghao 2012-03-25 21:13:52 -0700 (In reply to comment #4) > (From update of attachment 133443 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133443&action=review > > >> Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:149 > >> + self._graphics_type = 'gpu' > > > > Does graphics_type still exist? It seems like it should be removed now that the GPU configuration no longer exists. > > No, it doesn't still exist. > > > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:238 > > + return [] > > To be more future-proof, you might want to call the base class and then remove the gpu suites, in case other suites get added down the road. I guess Adam is right. It's gone. We fall behind trunk some way, and apparently I need to rework this change. But anyway, I'd like to hear your opinion first on issues we come across. As we always enable hardware gpu in our final build, we only need to run tests once in both real and virtual suites. 2 choices: - Run these tests in real suites. Return [] for virtual suites. But we can not inherit baselines in platform/chromium-*/platform/chromium/virtual/gpu/, and can not reuse expectations like BUGWK63933 : platform/chromium/virtual/gpu/fast/canvas/canvas-webkitLineDash.html = FAIL . - Run these tests in virtual suites. This seems more natural to me. I can use skipped_tests() to skip these tests in real suites? Which way looks better to you? 587450 6 dpranke 2012-03-25 22:28:51 -0700 (In reply to comment #5) > As we always enable hardware gpu in our final build, we only need to run tests once in both real and virtual suites. 2 choices: > - Run these tests in real suites. Return [] for virtual suites. But we can not inherit baselines in platform/chromium-*/platform/chromium/virtual/gpu/, and can not reuse expectations like > BUGWK63933 : platform/chromium/virtual/gpu/fast/canvas/canvas-webkitLineDash.html = FAIL . > > - Run these tests in virtual suites. This seems more natural to me. I can use skipped_tests() to skip these tests in real suites? > > Which way looks better to you? I think you are right that you should do whichever is likely to let you reuse more baselines and expectations. Running them as virtual suites -- which at least means they have the same "hardware accelerated" usage as the other ports -- seems perfectly reasonable to me. Sorry for the inconvenience; it's unfortunate that you're trying to merge things at the same time we were reworking the gpu ports, but at least that's done and I don't foresee anything similar on the horizon that should affect you. 588604 7 133989 zhenghao 2012-03-26 23:51:48 -0700 Created attachment 133989 Patch 588606 8 zhenghao 2012-03-26 23:54:49 -0700 (In reply to comment #2) > (From update of attachment 133443 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133443&action=review > > Thanks! Some drive-by nits. > > > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:1 > > +#!/usr/bin/env python > > Do we invoke chromium_android.py directly? I don't think this is necessary if it's just being imported elsewhere. > All scripts under webkitpy have such head. I just don't want android port to be an exception. > > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:137 > > + 'chromium-gpu', > > These names correspond to directories in LayoutTests/platform/. All the chromium-gpu-* directories got removed in r110517. > Done. > > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:148 > > + # The chromium-android port always uses the hardware GPU code path, > Done. > textual nit: "The Chromium port for Android always uses the hardware GPU path." > > > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:237 > > + # All tests run in hardware gpu path, so don't need to run virtual gpu tests. > > textual nit: "All tests are being run in the hardware GPU path, so there is no need to run virtual GPU tests." Removed. 588609 9 zhenghao 2012-03-27 00:02:57 -0700 (In reply to comment #7) > Created an attachment (id=133989) [details] > Patch I use skipped_tests(), which is not used by chromium port, because I find adding a line like 'WONTFIX ANDROID SKIP : fast/canvas = PASS FAIL' is not enough. More specific rules like 'BUGWEBGL : fast/canvas/webgl/read-pixels-test.html = TIMEOUT' would override the general SKIP rule, so the tests are still run. I have to override again using specific platform rules like 'WONTFIX ANDROID SKIP : fast/canvas/webgl/read-pixels-test.html = TIMEOUT'. This is annoying because we have to add one SKIP rule for Android each time we add one rule for canvas tests. 589192 10 133989 dpranke 2012-03-27 13:07:28 -0700 Comment on attachment 133989 Patch I think using skipped_tests in this case is fine. 589694 11 133989 webkit.review.bot 2012-03-28 00:05:24 -0700 Comment on attachment 133989 Patch Clearing flags on attachment: 133989 Committed r112368: <http://trac.webkit.org/changeset/112368> 589695 12 webkit.review.bot 2012-03-28 00:05:29 -0700 All reviewed patches have been landed. Closing bug. 610789 13 133989 wangxianzhu 2012-04-26 11:56:58 -0700 Comment on attachment 133989 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133989&action=review > Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:1 > +#!/usr/bin/env python Just FYI: I think abarth intentionally left this out in the first check-in. The magic is useless in a file that is not excutable itself. 133443 2012-03-23 01:33:00 -0700 2012-03-26 23:51:42 -0700 Patch bug-82033-20120323163258.patch text/plain 2675 zhenghao U3VidmVyc2lvbiBSZXZpc2lvbjogMTExODM1CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggZWQ2Zjg0YTU5MGUxMjA1Yjc1ZDI0MjdmNTY5ZTE1MThm OTdhYmY1YS4uYzk2NDkxM2Q2NDEyOTljOWNmY2QwNGZjYzAyYjRlOTRlMjg5N2I5MyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2 IEBACisyMDEyLTAzLTIzICBIYW8gWmhlbmcgIDx6aGVuZ2hhb0BjaHJvbWl1bS5vcmc+CisKKyAg ICAgICAgTWFrZSBjaHJvbWl1bSBhbmRyb2lkIHBvcnQgdXNlIGhhcmR3YXJlIGdwdSBwYXRoIGRl ZmF1bHQuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04 MjAzMworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICog U2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1bV9hbmRyb2lkLnB5Ogor ICAgICAgICAoQ2hyb21pdW1BbmRyb2lkUG9ydCk6CisgICAgICAgIChDaHJvbWl1bUFuZHJvaWRQ b3J0Ll9faW5pdF9fKTogU2V0IGFsbCBuZWNlc3Nhcnkgb3B0aW9ucy4KKyAgICAgICAgKENocm9t aXVtQW5kcm9pZFBvcnQudmlydHVhbF90ZXN0X3N1aXRlcyk6IERvIG5vdCB1c2UgdmlydHVhbCB0 ZXN0CisgICAgICAgIHN1aXRlcy4KKwogMjAxMi0wMy0yMyAgR3J6ZWdvcnogQ3phamtvd3NraSAg PGcuY3phamtvd3NraUBzYW1zdW5nLmNvbT4KIAogICAgICAgICBbRUZMXVtEUlRdIEltcGxlbWVu dCBMYXlvdXRUZXN0Q29udHJvbGxlcidzIG1ldGhvZHMgcmVsYXRlZCB3aXRoIGVkaXRpbmcuCmRp ZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Nocm9t aXVtX2FuZHJvaWQucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0 L2Nocm9taXVtX2FuZHJvaWQucHkKaW5kZXggMGNiNmQzMWJkNWFlYThiZjkyOWJmZTFhZjA2Zjc0 N2FiODMyNGIyZC4uNGZmZDkzMTg4ZDU0NThhZjhhMTQzZWUxYWFhNDgyYmRmZmYwZTMyNCAxMDA2 NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1 bV9hbmRyb2lkLnB5CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3Bv cnQvY2hyb21pdW1fYW5kcm9pZC5weQpAQCAtMSwzICsxLDQgQEAKKyMhL3Vzci9iaW4vZW52IHB5 dGhvbgogIyBDb3B5cmlnaHQgKEMpIDIwMTIgR29vZ2xlIEluYy4gQWxsIHJpZ2h0cyByZXNlcnZl ZC4KICMKICMgUmVkaXN0cmlidXRpb24gYW5kIHVzZSBpbiBzb3VyY2UgYW5kIGJpbmFyeSBmb3Jt cywgd2l0aCBvciB3aXRob3V0CkBAIC0xMzEsNiArMTMyLDkgQEAgY2xhc3MgQ2hyb21pdW1BbmRy b2lkUG9ydChjaHJvbWl1bS5DaHJvbWl1bVBvcnQpOgogCiAgICAgRkFMTEJBQ0tfUEFUSFMgPSBb CiAgICAgICAgICdjaHJvbWl1bS1hbmRyb2lkJywKKyAgICAgICAgJ2Nocm9taXVtLWdwdS1saW51 eCcsCisgICAgICAgICdjaHJvbWl1bS1ncHUtd2luJywKKyAgICAgICAgJ2Nocm9taXVtLWdwdScs CiAgICAgICAgICdjaHJvbWl1bS1saW51eCcsCiAgICAgICAgICdjaHJvbWl1bS13aW4nLAogICAg ICAgICAnY2hyb21pdW0nLApAQCAtMTQxLDYgKzE0NSwxMiBAQCBjbGFzcyBDaHJvbWl1bUFuZHJv aWRQb3J0KGNocm9taXVtLkNocm9taXVtUG9ydCk6CiAgICAgZGVmIF9faW5pdF9fKHNlbGYsIGhv c3QsIHBvcnRfbmFtZSwgKiprd2FyZ3MpOgogICAgICAgICBjaHJvbWl1bS5DaHJvbWl1bVBvcnQu X19pbml0X18oc2VsZiwgaG9zdCwgcG9ydF9uYW1lLCAqKmt3YXJncykKIAorICAgICAgICAjIFRo ZSBjaHJvbWl1bS1hbmRyb2lkIHBvcnQgYWx3YXlzIHVzZXMgdGhlIGhhcmR3YXJlIEdQVSBjb2Rl IHBhdGgsCisgICAgICAgIHNlbGYuX2dyYXBoaWNzX3R5cGUgPSAnZ3B1JworICAgICAgICBzZWxm Ll9vcHRpb25zLmFjY2VsZXJhdGVkXzJkX2NhbnZhcyA9IFRydWUKKyAgICAgICAgc2VsZi5fb3B0 aW9ucy5hY2NlbGVyYXRlZF92aWRlbyA9IFRydWUKKyAgICAgICAgc2VsZi5fb3B0aW9ucy5lbmFi bGVfaGFyZHdhcmVfZ3B1ID0gVHJ1ZQorCiAgICAgICAgIHNlbGYuX29wZXJhdGluZ19zeXN0ZW0g PSAnYW5kcm9pZCcKICAgICAgICAgc2VsZi5fdmVyc2lvbiA9ICdpY2VjcmVhbXNhbmR3aWNoJwog ICAgICAgICBzZWxmLl9vcmlnaW5hbF9nb3Zlcm5vciA9IE5vbmUKQEAgLTIyMyw2ICsyMzMsMTAg QEAgY2xhc3MgQ2hyb21pdW1BbmRyb2lkUG9ydChjaHJvbWl1bS5DaHJvbWl1bVBvcnQpOgogICAg ICAgICAjIHVzZWZ1bCBmb3IgZGVidWdnaW5nIGFuZCBkbyBubyBoYXJtIHRvIHN1YnNlcXVlbnQg dGVzdHMuCiAgICAgICAgIHNlbGYuX3RlYXJkb3duX3BlcmZvcm1hbmNlKCkKIAorICAgIGRlZiB2 aXJ0dWFsX3Rlc3Rfc3VpdGVzKHNlbGYpOgorICAgICAgICAjIEFsbCB0ZXN0cyBydW4gaW4gaGFy ZHdhcmUgZ3B1IHBhdGgsIHNvIGRvbid0IG5lZWQgdG8gcnVuIHZpcnR1YWwgZ3B1IHRlc3RzLgor ICAgICAgICByZXR1cm4gW10KKwogICAgIGRlZiBfYnVpbGRfcGF0aChzZWxmLCAqY29tcHMpOgog ICAgICAgICByZXR1cm4gc2VsZi5faG9zdF9wb3J0Ll9idWlsZF9wYXRoKCpjb21wcykKIAo= 133989 2012-03-26 23:51:48 -0700 2012-04-26 11:56:58 -0700 Patch bug-82033-20120327145146.patch text/plain 2439 zhenghao U3VidmVyc2lvbiBSZXZpc2lvbjogMTEyMjA5CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggZTg3YWEwMDA4YjEzNTdmNTQ2ZWE4MGUxMzZkMGQ4OTQ3 YmQyYjQ2Zi4uMjZlZWM3NjA1YjZmY2UzMmJhN2M2ODM0NGQ0OTAyYTg3MjlmMzY4NSAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2 IEBACisyMDEyLTAzLTI2ICBIYW8gWmhlbmcgIDx6aGVuZ2hhb0BjaHJvbWl1bS5vcmc+CisKKyAg ICAgICAgTWFrZSBDaHJvbWl1bSBwb3J0IGZvciBBbmRyb2lkIHVzZSBoYXJkd2FyZSBncHUgcGF0 aCBkZWZhdWx0LgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9ODIwMzMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBTa2lwIGNhbnZhcyB0ZXN0cywgd2hpY2ggYXJlIHJ1biBpbiB2aXJ0dWFsIGdwdSB0ZXN0IHN1 aXRlcy4KKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvY2hy b21pdW1fYW5kcm9pZC5weToKKyAgICAgICAgKENocm9taXVtQW5kcm9pZFBvcnQuX19pbml0X18p OgorICAgICAgICAoQ2hyb21pdW1BbmRyb2lkUG9ydC5za2lwcGVkX3Rlc3RzKToKKwogMjAxMi0w My0yNiAgU2NvdHQgQnllciAgPHNjb3R0YnllckBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgRW5h YmxlIGxheW91dCB0ZXN0aW5nIG9mIHRoZSBzY3JvbGwgYW5pbWF0b3IuCmRpZmYgLS1naXQgYS9U b29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Nocm9taXVtX2FuZHJvaWQu cHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Nocm9taXVtX2Fu ZHJvaWQucHkKaW5kZXggNjFhMjUzYTk1ZjAxMjY0YzVkNWJhNjE3NzE3Y2M4MDRlOWZhNDk5My4u NDRkNTg1MTQ0ZTdjMDY0OWRiNWY0NzMyYmIyM2E0NzZiYzkyM2Q5NyAxMDA2NDQKLS0tIGEvVG9v bHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1bV9hbmRyb2lkLnB5 CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvY2hyb21pdW1f YW5kcm9pZC5weQpAQCAtMSwzICsxLDQgQEAKKyMhL3Vzci9iaW4vZW52IHB5dGhvbgogIyBDb3B5 cmlnaHQgKEMpIDIwMTIgR29vZ2xlIEluYy4gQWxsIHJpZ2h0cyByZXNlcnZlZC4KICMKICMgUmVk aXN0cmlidXRpb24gYW5kIHVzZSBpbiBzb3VyY2UgYW5kIGJpbmFyeSBmb3Jtcywgd2l0aCBvciB3 aXRob3V0CkBAIC0yOCw2ICsyOSw3IEBACiAKIGltcG9ydCBsb2dnaW5nCiAKK2Zyb20gd2Via2l0 cHkubGF5b3V0X3Rlc3RzLnBvcnQgaW1wb3J0IGJhc2UKIGZyb20gd2Via2l0cHkubGF5b3V0X3Rl c3RzLnBvcnQgaW1wb3J0IGNocm9taXVtCiBmcm9tIHdlYmtpdHB5LmxheW91dF90ZXN0cy5wb3J0 IGltcG9ydCBmYWN0b3J5CiAKQEAgLTE0MSw2ICsxNDMsOSBAQCBjbGFzcyBDaHJvbWl1bUFuZHJv aWRQb3J0KGNocm9taXVtLkNocm9taXVtUG9ydCk6CiAgICAgZGVmIF9faW5pdF9fKHNlbGYsIGhv c3QsIHBvcnRfbmFtZSwgKiprd2FyZ3MpOgogICAgICAgICBjaHJvbWl1bS5DaHJvbWl1bVBvcnQu X19pbml0X18oc2VsZiwgaG9zdCwgcG9ydF9uYW1lLCAqKmt3YXJncykKIAorICAgICAgICAjIFRo ZSBDaHJvbWl1bSBwb3J0IGZvciBBbmRyb2lkIGFsd2F5cyB1c2VzIHRoZSBoYXJkd2FyZSBHUFUg cGF0aC4KKyAgICAgICAgc2VsZi5fb3B0aW9ucy5lbmFibGVfaGFyZHdhcmVfZ3B1ID0gVHJ1ZQor CiAgICAgICAgIHNlbGYuX29wZXJhdGluZ19zeXN0ZW0gPSAnYW5kcm9pZCcKICAgICAgICAgc2Vs Zi5fdmVyc2lvbiA9ICdpY2VjcmVhbXNhbmR3aWNoJwogICAgICAgICBzZWxmLl9vcmlnaW5hbF9n b3Zlcm5vciA9IE5vbmUKQEAgLTIyMCw2ICsyMjUsMTMgQEAgY2xhc3MgQ2hyb21pdW1BbmRyb2lk UG9ydChjaHJvbWl1bS5DaHJvbWl1bVBvcnQpOgogICAgICAgICAjIHVzZWZ1bCBmb3IgZGVidWdn aW5nIGFuZCBkbyBubyBoYXJtIHRvIHN1YnNlcXVlbnQgdGVzdHMuCiAgICAgICAgIHNlbGYuX3Rl YXJkb3duX3BlcmZvcm1hbmNlKCkKIAorICAgIGRlZiBza2lwcGVkX3Rlc3RzKHNlbGYsIHRlc3Rf bGlzdCk6CisgICAgICAgIHJldHVybiBiYXNlLlBvcnQuX3JlYWxfdGVzdHMoc2VsZiwgWworICAg ICAgICAgICAgIyBDYW52YXMgdGVzdHMgYXJlIHJ1biBhcyB2aXJ0dWFsIGdwdSB0ZXN0cy4KKyAg ICAgICAgICAgICdmYXN0L2NhbnZhcycsCisgICAgICAgICAgICAnY2FudmFzL3BoaWxpcCcsCisg ICAgICAgIF0pCisKICAgICBkZWYgX2J1aWxkX3BhdGgoc2VsZiwgKmNvbXBzKToKICAgICAgICAg cmV0dXJuIHNlbGYuX2hvc3RfcG9ydC5fYnVpbGRfcGF0aCgqY29tcHMpCiAK