Remove unsafe mode to enable es3 api by default for WebGL2 and ES3 context

Remove unsafe mode to enable es3 api by default for WebGL2 and ES3 context. 

The blink side UnsafeES3APIs runtime flag has been removed by https://crrev.com/2451943002/.
So, with this patch applied, the ES3 APIs will be finally enabled by default for WebGL2 in Chromium!

BUG=654787
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

[yunchao, 2016-10-24 07:46:00]

Description was changed from

==========
remove unsafe mode

BUG=
==========

to

==========
remove unsafe mode

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[yunchao, 2016-10-25 09:17:07]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-25 09:17:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-25 09:19:12]

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

[commit-bot: I haz the power, 2016-10-25 09:19:14]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[yunchao, 2016-10-25 10:08:04]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-25 10:08:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-25 11:00:58]

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

[commit-bot: I haz the power, 2016-10-25 11:00:59]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[yunchao, 2016-10-25 13:50:45]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-25 13:51:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-25 14:11:26]

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

[commit-bot: I haz the power, 2016-10-25 14:11:27]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[yunchao, 2016-10-25 14:20:00]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-25 14:20:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yunchao, 2016-10-25 14:26:32]

Patchset #4 (id:60001) has been deleted

[yunchao, 2016-10-25 14:55:34]

Description was changed from

==========
remove unsafe mode

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Remove unsafe mode to enable es3 api by default for WebGL2 and ES3 context

BUG=654787
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[yunchao, 2016-10-25 14:55:34]

yunchao.he@intel.com changed reviewers:
+ kainino@chromium.org, kbr@chromium.org, qiankun.miao@intel.com,
zmo@chromium.org

[commit-bot: I haz the power, 2016-10-25 15:12:21]

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

[commit-bot: I haz the power, 2016-10-25 15:12:22]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Zhenyao Mo, 2016-10-25 16:52:05]

https://codereview.chromium.org/2444813002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGL2RenderingContext.cpp (left):

https://codereview.chromium.org/2444813002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGL2RenderingContext.cpp:50: if
(!contextProvider)
I am wondering if only with the blink side change (so UnsafeES3APIs still
effective in command buffer but not in blink), will this just return null if
--enable-unsafe-es3-apis isn't passed in?

If yes, then we can land the blink side change first.

Yunchao, can you verify the above question and if yes, can you split the blink
side work to a separate CL and then we can land it?

[yunchao, 2016-10-26 00:27:34]

On 2016/10/25 16:52:05, Zhenyao Mo wrote:
>
https://codereview.chromium.org/2444813002/diff/80001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/webgl/WebGL2RenderingContext.cpp
(left):
> 
>
https://codereview.chromium.org/2444813002/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/webgl/WebGL2RenderingContext.cpp:50: if
> (!contextProvider)
> I am wondering if only with the blink side change (so UnsafeES3APIs still
> effective in command buffer but not in blink), will this just return null if
> --enable-unsafe-es3-apis isn't passed in?
> 
> If yes, then we can land the blink side change first.
> 
> Yunchao, can you verify the above question and if yes, can you split the blink
> side work to a separate CL and then we can land it?

Zhenyao, do you mean 1) or 2) below? I can have a try. 
1) still keep command line option --enable-unsafe-es3-apis in command buffer,
only remove it from blink webgl side. And we still need this command line to run
webgl2 CTS. 
2) keep --enable-unsafe-es3-apis related code in command buffer only, we can run
webgl2 CTS without --enable-unsafe-es3-apis command line option.

[Zhenyao Mo, 2016-10-26 00:35:26]

On 2016/10/26 00:27:34, yunchao wrote:
> On 2016/10/25 16:52:05, Zhenyao Mo wrote:
> >
>
https://codereview.chromium.org/2444813002/diff/80001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/modules/webgl/WebGL2RenderingContext.cpp
> (left):
> > 
> >
>
https://codereview.chromium.org/2444813002/diff/80001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/modules/webgl/WebGL2RenderingContext.cpp:50: if
> > (!contextProvider)
> > I am wondering if only with the blink side change (so UnsafeES3APIs still
> > effective in command buffer but not in blink), will this just return null if
> > --enable-unsafe-es3-apis isn't passed in?
> > 
> > If yes, then we can land the blink side change first.
> > 
> > Yunchao, can you verify the above question and if yes, can you split the
blink
> > side work to a separate CL and then we can land it?
> 
> Zhenyao, do you mean 1) or 2) below? I can have a try. 
> 1) still keep command line option --enable-unsafe-es3-apis in command buffer,
> only remove it from blink webgl side. And we still need this command line to
run
> webgl2 CTS. 
> 2) keep --enable-unsafe-es3-apis related code in command buffer only, we can
run
> webgl2 CTS without --enable-unsafe-es3-apis command line option.

I meant 1)

[yunchao, 2016-10-26 03:27:09]

Thanks for your suggestion, Zhenyao. The separated CL is uploaded to
https://codereview.chromium.org/2451943002/, could you take a look?

https://codereview.chromium.org/2444813002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGL2RenderingContext.cpp (left):

https://codereview.chromium.org/2444813002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGL2RenderingContext.cpp:50: if
(!contextProvider)
On 2016/10/25 16:52:05, Zhenyao Mo wrote:
> I am wondering if only with the blink side change (so UnsafeES3APIs still
> effective in command buffer but not in blink), will this just return null if
> --enable-unsafe-es3-apis isn't passed in?
> 
> If yes, then we can land the blink side change first.
> 
> Yunchao, can you verify the above question and if yes, can you split the blink
> side work to a separate CL and then we can land it?

I tried your suggestion in my local Linux desktop, the answer is "yes".
contextProvider would be null. It fails to be created because GLES2DecoderImpl
would fail during initiation when --enable-unsafe-es3-apis is not passed in.

Please see the change at https://codereview.chromium.org/2451943002/

[yunchao, 2016-10-27 09:28:47]

Patchset #3 (id:40001) has been deleted

[yunchao, 2016-10-27 09:29:04]

Patchset #2 (id:20001) has been deleted

[yunchao, 2016-10-27 09:29:13]

Patchset #1 (id:1) has been deleted

[yunchao, 2016-10-27 10:08:20]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 10:08:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 11:01:00]

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

[commit-bot: I haz the power, 2016-10-27 11:01:01]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[yunchao, 2016-10-27 14:04:24]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 14:04:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yunchao, 2016-10-27 14:04:58]

Patchset #3 (id:120001) has been deleted

[yunchao, 2016-10-27 14:07:36]

Patchset #3 (id:140001) has been deleted

[yunchao, 2016-10-27 14:08:30]

Patchset #4 (id:180001) has been deleted

[yunchao, 2016-10-27 14:08:58]

Patchset #3 (id:160001) has been deleted

[yunchao, 2016-10-27 14:47:27]

Ken and Zhenyao, I separated my local patch into several patch sets (ps2 - ps6).
I added a number for every patch set, which was the tasks I listed in
https://crbug.com/654787/. The last patch set should be unnecessary after
https://crrev.com/2451943002/ is landed. I added it here to run the bots. 

My local tests showed that all WebGL 1 and WebGL 2 CTS can pass in MacOS. But a
couple of tests failed in Windows and Linux. I think we can mark them failed in
webgl2_conformance_test_expectations.py, and fixed them later. 

Could you take a look for patch set 2 to patch set 5. Thanks a lot!

[yunchao, 2016-10-27 15:06:21]

On 2016/10/27 14:47:27, yunchao wrote:
> Ken and Zhenyao, I separated my local patch into several patch sets (ps2 -
ps6).
> I added a number for every patch set, which was the tasks I listed in
> https://crbug.com/654787/. The last patch set should be unnecessary after
> https://crrev.com/2451943002/ is landed. I added it here to run the bots. 
> 
> My local tests showed that all WebGL 1 and WebGL 2 CTS can pass in MacOS. But
a
> couple of tests failed in Windows and Linux. I think we can mark them failed
in
> webgl2_conformance_test_expectations.py, and fixed them later. 
> 
> Could you take a look for patch set 2 to patch set 5. Thanks a lot!

BTW, please feel free to pick up my code and submit a new now. 
I just eager to see the result against all trybots when we remove 'unsafe' mode
and enable ES3 APIs for ES3 and WebGL2 context by default.

[commit-bot: I haz the power, 2016-10-27 15:20:03]

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

[commit-bot: I haz the power, 2016-10-27 15:20:04]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[Ken Russell (switch to Gerrit), 2016-10-28 00:53:54]

On 2016/10/27 15:06:21, yunchao wrote:
> On 2016/10/27 14:47:27, yunchao wrote:
> > Ken and Zhenyao, I separated my local patch into several patch sets (ps2 -
> ps6).
> > I added a number for every patch set, which was the tasks I listed in
> > https://crbug.com/654787/. The last patch set should be unnecessary after
> > https://crrev.com/2451943002/ is landed. I added it here to run the bots. 
> > 
> > My local tests showed that all WebGL 1 and WebGL 2 CTS can pass in MacOS.
But
> a
> > couple of tests failed in Windows and Linux. I think we can mark them failed
> in
> > webgl2_conformance_test_expectations.py, and fixed them later. 
> > 
> > Could you take a look for patch set 2 to patch set 5. Thanks a lot!
> 
> BTW, please feel free to pick up my code and submit a new now. 
> I just eager to see the result against all trybots when we remove 'unsafe'
mode
> and enable ES3 APIs for ES3 and WebGL2 context by default.

Thanks Yunchao for this work. Kai and Mo are actively working on this so they
should review and comment.

[Kai Ninomiya, 2016-10-28 01:06:46]

On 2016/10/28 00:53:54, Ken Russell wrote:
> On 2016/10/27 15:06:21, yunchao wrote:
> > On 2016/10/27 14:47:27, yunchao wrote:
> > > Ken and Zhenyao, I separated my local patch into several patch sets (ps2 -
> > ps6).
> > > I added a number for every patch set, which was the tasks I listed in
> > > https://crbug.com/654787/. The last patch set should be unnecessary after
> > > https://crrev.com/2451943002/ is landed. I added it here to run the bots. 
> > > 
> > > My local tests showed that all WebGL 1 and WebGL 2 CTS can pass in MacOS.
> But
> > a
> > > couple of tests failed in Windows and Linux. I think we can mark them
failed
> > in
> > > webgl2_conformance_test_expectations.py, and fixed them later. 
> > > 
> > > Could you take a look for patch set 2 to patch set 5. Thanks a lot!
> > 
> > BTW, please feel free to pick up my code and submit a new now. 
> > I just eager to see the result against all trybots when we remove 'unsafe'
> mode
> > and enable ES3 APIs for ES3 and WebGL2 context by default.
> 
> Thanks Yunchao for this work. Kai and Mo are actively working on this so they
> should review and comment.

I meant to reply to this earlier, but I lost track of the tab.

Yunchao, thank you for splitting up the CL. I think it should be very helpful,
especially PS2-PS5. We are doing the work in a slightly different order. Right
now I'm working on manually auditing the command buffer validation, and Mo will
join me on that soon. After that I think we will be applying most of your
changes from PS2-PS5. Will comment here at that point.

[yunchao, 2016-10-28 02:54:54]

Patchset #6 (id:260001) has been deleted

[yunchao, 2016-10-28 02:55:03]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 02:55:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 04:11:44]

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

[commit-bot: I haz the power, 2016-10-28 04:11:45]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[yunchao, 2016-10-28 07:30:37]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 07:30:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 08:21:21]

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

[commit-bot: I haz the power, 2016-10-28 08:21:22]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[yunchao, 2016-10-28 08:43:04]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 08:43:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yunchao, 2016-10-28 09:17:02]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 09:17:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 10:14:02]

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

[commit-bot: I haz the power, 2016-10-28 10:14:04]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[yunchao, 2016-10-28 14:19:26]

Patchset #6 (id:280001) has been deleted

[yunchao, 2016-10-28 14:24:08]

On 2016/10/28 01:06:46, Kai Ninomiya wrote:
> On 2016/10/28 00:53:54, Ken Russell wrote:
> > On 2016/10/27 15:06:21, yunchao wrote:
> > > On 2016/10/27 14:47:27, yunchao wrote:
> > > > Ken and Zhenyao, I separated my local patch into several patch sets (ps2
-
> > > ps6).
> > > > I added a number for every patch set, which was the tasks I listed in
> > > > https://crbug.com/654787/. The last patch set should be unnecessary
after
> > > > https://crrev.com/2451943002/ is landed. I added it here to run the
bots. 
> > > > 
> > > > My local tests showed that all WebGL 1 and WebGL 2 CTS can pass in
MacOS.
> > But
> > > a
> > > > couple of tests failed in Windows and Linux. I think we can mark them
> failed
> > > in
> > > > webgl2_conformance_test_expectations.py, and fixed them later. 
> > > > 
> > > > Could you take a look for patch set 2 to patch set 5. Thanks a lot!
> > > 
> > > BTW, please feel free to pick up my code and submit a new now. 
> > > I just eager to see the result against all trybots when we remove 'unsafe'
> > mode
> > > and enable ES3 APIs for ES3 and WebGL2 context by default.
> > 
> > Thanks Yunchao for this work. Kai and Mo are actively working on this so
they
> > should review and comment.
> 
> I meant to reply to this earlier, but I lost track of the tab.
> 
> Yunchao, thank you for splitting up the CL. I think it should be very helpful,
> especially PS2-PS5. We are doing the work in a slightly different order. Right
> now I'm working on manually auditing the command buffer validation, and Mo
will
> join me on that soon. After that I think we will be applying most of your
> changes from PS2-PS5. Will comment here at that point.

OK, Thank you, Kai. 

With patch set 6, now only 3 webgl tests failed, and the root cause is the same:
got GL_INVALID_OPERATION during context switch.

[yunchao, 2016-10-28 15:03:28]

On 2016/10/28 14:24:08, yunchao wrote:
> On 2016/10/28 01:06:46, Kai Ninomiya wrote:
> > On 2016/10/28 00:53:54, Ken Russell wrote:
> > > On 2016/10/27 15:06:21, yunchao wrote:
> > > > On 2016/10/27 14:47:27, yunchao wrote:
> > > > > Ken and Zhenyao, I separated my local patch into several patch sets
(ps2
> -
> > > > ps6).
> > > > > I added a number for every patch set, which was the tasks I listed in
> > > > > https://crbug.com/654787/. The last patch set should be unnecessary
> after
> > > > > https://crrev.com/2451943002/ is landed. I added it here to run the
> bots. 
> > > > > 
> > > > > My local tests showed that all WebGL 1 and WebGL 2 CTS can pass in
> MacOS.
> > > But
> > > > a
> > > > > couple of tests failed in Windows and Linux. I think we can mark them
> > failed
> > > > in
> > > > > webgl2_conformance_test_expectations.py, and fixed them later. 
> > > > > 
> > > > > Could you take a look for patch set 2 to patch set 5. Thanks a lot!
> > > > 
> > > > BTW, please feel free to pick up my code and submit a new now. 
> > > > I just eager to see the result against all trybots when we remove
'unsafe'
> > > mode
> > > > and enable ES3 APIs for ES3 and WebGL2 context by default.
> > > 
> > > Thanks Yunchao for this work. Kai and Mo are actively working on this so
> they
> > > should review and comment.
> > 
> > I meant to reply to this earlier, but I lost track of the tab.
> > 
> > Yunchao, thank you for splitting up the CL. I think it should be very
helpful,
> > especially PS2-PS5. We are doing the work in a slightly different order.
Right
> > now I'm working on manually auditing the command buffer validation, and Mo
> will
> > join me on that soon. After that I think we will be applying most of your
> > changes from PS2-PS5. Will comment here at that point.

(Sorry, send it again to make this reply clearer). 

OK, Thank you, Kai. 
 
With patch set 6 applied, now only 3 webgl tests failed on Linux trybots, and
all of the other bots passed!
The root cause of these 3 failures is the same:
got GL_INVALID_OPERATION during context switch. 

Zhenyao, Ken and Kai, any suggestions about fixing this failure?

[yunchao, 2016-10-28 15:06:51]

Description was changed from

==========
Remove unsafe mode to enable es3 api by default for WebGL2 and ES3 context

BUG=654787
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Remove unsafe mode to enable es3 api by default for WebGL2 and ES3 context. 

The blink side UnsafeES3APIs runtime flag is remove by
https://crrev.com/2451943002/.
So, with this patch applied, the ES3 APIs will be finally enabled by default for
WebGL2!

BUG=654787
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[yunchao, 2016-10-28 15:07:29]

Description was changed from

==========
Remove unsafe mode to enable es3 api by default for WebGL2 and ES3 context. 

The blink side UnsafeES3APIs runtime flag is remove by
https://crrev.com/2451943002/.
So, with this patch applied, the ES3 APIs will be finally enabled by default for
WebGL2!

BUG=654787
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Remove unsafe mode to enable es3 api by default for WebGL2 and ES3 context. 

The blink side UnsafeES3APIs runtime flag has been removed by
https://crrev.com/2451943002/.
So, with this patch applied, the ES3 APIs will be finally enabled by default for
WebGL2 in Chromium!

BUG=654787
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Kai Ninomiya, 2016-10-28 16:29:50]

The CQ bit was checked by kainino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 16:30:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Kai Ninomiya, 2016-10-28 16:40:02]

On 2016/10/28 15:03:28, yunchao wrote:
> On 2016/10/28 14:24:08, yunchao wrote:
> > On 2016/10/28 01:06:46, Kai Ninomiya wrote:
> > > On 2016/10/28 00:53:54, Ken Russell wrote:
> > > > On 2016/10/27 15:06:21, yunchao wrote:
> > > > > On 2016/10/27 14:47:27, yunchao wrote:
> > > > > > Ken and Zhenyao, I separated my local patch into several patch sets
> (ps2
> > -
> > > > > ps6).
> > > > > > I added a number for every patch set, which was the tasks I listed
in
> > > > > > https://crbug.com/654787/. The last patch set should be unnecessary
> > after
> > > > > > https://crrev.com/2451943002/ is landed. I added it here to run the
> > bots. 
> > > > > > 
> > > > > > My local tests showed that all WebGL 1 and WebGL 2 CTS can pass in
> > MacOS.
> > > > But
> > > > > a
> > > > > > couple of tests failed in Windows and Linux. I think we can mark
them
> > > failed
> > > > > in
> > > > > > webgl2_conformance_test_expectations.py, and fixed them later. 
> > > > > > 
> > > > > > Could you take a look for patch set 2 to patch set 5. Thanks a lot!
> > > > > 
> > > > > BTW, please feel free to pick up my code and submit a new now. 
> > > > > I just eager to see the result against all trybots when we remove
> 'unsafe'
> > > > mode
> > > > > and enable ES3 APIs for ES3 and WebGL2 context by default.
> > > > 
> > > > Thanks Yunchao for this work. Kai and Mo are actively working on this so
> > they
> > > > should review and comment.
> > > 
> > > I meant to reply to this earlier, but I lost track of the tab.
> > > 
> > > Yunchao, thank you for splitting up the CL. I think it should be very
> helpful,
> > > especially PS2-PS5. We are doing the work in a slightly different order.
> Right
> > > now I'm working on manually auditing the command buffer validation, and Mo
> > will
> > > join me on that soon. After that I think we will be applying most of your
> > > changes from PS2-PS5. Will comment here at that point.
> 
> (Sorry, send it again to make this reply clearer). 
> 
> OK, Thank you, Kai. 
>  
> With patch set 6 applied, now only 3 webgl tests failed on Linux trybots, and
> all of the other bots passed!
> The root cause of these 3 failures is the same:
> got GL_INVALID_OPERATION during context switch. 
> 
> Zhenyao, Ken and Kai, any suggestions about fixing this failure?

I haven't looked at your patch in much detail, but it's strange to me that this
change could trigger those failures. I'm rerunning the dry run to see what
happens. If they pass this time, we will have to figure out whether it was your
change that made it flaky.

WebglConformance_deqp_functional_gles3_transformfeedback_random_interleaved_triangles
in particular looks like it flaked - it seems to have passed once and failed
once on the same patch:
*
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional...
*
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional...
But it's the same error, so it's probably the same issue.

[Kai Ninomiya, 2016-10-28 17:19:37]

On 2016/10/28 16:40:02, Kai Ninomiya wrote:
> On 2016/10/28 15:03:28, yunchao wrote:
> > On 2016/10/28 14:24:08, yunchao wrote:
> > > On 2016/10/28 01:06:46, Kai Ninomiya wrote:
> > > > On 2016/10/28 00:53:54, Ken Russell wrote:
> > > > > On 2016/10/27 15:06:21, yunchao wrote:
> > > > > > On 2016/10/27 14:47:27, yunchao wrote:
> > > > > > > Ken and Zhenyao, I separated my local patch into several patch
sets
> > (ps2
> > > -
> > > > > > ps6).
> > > > > > > I added a number for every patch set, which was the tasks I listed
> in
> > > > > > > https://crbug.com/654787/. The last patch set should be
unnecessary
> > > after
> > > > > > > https://crrev.com/2451943002/ is landed. I added it here to run
the
> > > bots. 
> > > > > > > 
> > > > > > > My local tests showed that all WebGL 1 and WebGL 2 CTS can pass in
> > > MacOS.
> > > > > But
> > > > > > a
> > > > > > > couple of tests failed in Windows and Linux. I think we can mark
> them
> > > > failed
> > > > > > in
> > > > > > > webgl2_conformance_test_expectations.py, and fixed them later. 
> > > > > > > 
> > > > > > > Could you take a look for patch set 2 to patch set 5. Thanks a
lot!
> > > > > > 
> > > > > > BTW, please feel free to pick up my code and submit a new now. 
> > > > > > I just eager to see the result against all trybots when we remove
> > 'unsafe'
> > > > > mode
> > > > > > and enable ES3 APIs for ES3 and WebGL2 context by default.
> > > > > 
> > > > > Thanks Yunchao for this work. Kai and Mo are actively working on this
so
> > > they
> > > > > should review and comment.
> > > > 
> > > > I meant to reply to this earlier, but I lost track of the tab.
> > > > 
> > > > Yunchao, thank you for splitting up the CL. I think it should be very
> > helpful,
> > > > especially PS2-PS5. We are doing the work in a slightly different order.
> > Right
> > > > now I'm working on manually auditing the command buffer validation, and
Mo
> > > will
> > > > join me on that soon. After that I think we will be applying most of
your
> > > > changes from PS2-PS5. Will comment here at that point.
> > 
> > (Sorry, send it again to make this reply clearer). 
> > 
> > OK, Thank you, Kai. 
> >  
> > With patch set 6 applied, now only 3 webgl tests failed on Linux trybots,
and
> > all of the other bots passed!
> > The root cause of these 3 failures is the same:
> > got GL_INVALID_OPERATION during context switch. 
> > 
> > Zhenyao, Ken and Kai, any suggestions about fixing this failure?
> 
> I haven't looked at your patch in much detail, but it's strange to me that
this
> change could trigger those failures. I'm rerunning the dry run to see what
> happens. If they pass this time, we will have to figure out whether it was
your
> change that made it flaky.
> 
>
WebglConformance_deqp_functional_gles3_transformfeedback_random_interleaved_triangles
> in particular looks like it flaked - it seems to have passed once and failed
> once on the same patch:
> *
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional...
> *
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional...
> But it's the same error, so it's probably the same issue.

This time, only glsl_construct_mat4 failed.

https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional...

From looking at the failure history, it does look like at least these failures
are specific to this CL. But they are still slightly flaky.

[commit-bot: I haz the power, 2016-10-28 17:30:56]

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

[commit-bot: I haz the power, 2016-10-28 17:30:57]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[Kai Ninomiya, 2016-11-03 22:48:30]

I'm now going to close this in favor of my series of CLs:

Command buffer validation auditing:
https://codereview.chromium.org/2458523005
https://codereview.chromium.org/2461023002
https://codereview.chromium.org/2470623002
https://codereview.chromium.org/2466763002
GPU process flag removal:
https://codereview.chromium.org/2471853002
Enable WebGL 2 by default:
https://codereview.chromium.org/2473933002

Thanks yunchao for this patch, I used it to find a number of edits at different
places in the code which I had missed on my first run through.

[Ken Russell (switch to Gerrit), 2016-11-03 23:28:39]

On 2016/11/03 22:48:30, Kai Ninomiya wrote:
> I'm now going to close this in favor of my series of CLs:
> 
> Command buffer validation auditing:
> https://codereview.chromium.org/2458523005
> https://codereview.chromium.org/2461023002
> https://codereview.chromium.org/2470623002
> https://codereview.chromium.org/2466763002
> GPU process flag removal:
> https://codereview.chromium.org/2471853002
> Enable WebGL 2 by default:
> https://codereview.chromium.org/2473933002
> 
> Thanks yunchao for this patch, I used it to find a number of edits at
different
> places in the code which I had missed on my first run through.

Fantastic! Thanks Yunchao for your CL and Kai for following through on this.

[yunchao, 2016-11-04 05:20:43]

On 2016/11/03 23:28:39, Ken Russell wrote:
> On 2016/11/03 22:48:30, Kai Ninomiya wrote:
> > I'm now going to close this in favor of my series of CLs:
> > 
> > Command buffer validation auditing:
> > https://codereview.chromium.org/2458523005
> > https://codereview.chromium.org/2461023002
> > https://codereview.chromium.org/2470623002
> > https://codereview.chromium.org/2466763002
> > GPU process flag removal:
> > https://codereview.chromium.org/2471853002
> > Enable WebGL 2 by default:
> > https://codereview.chromium.org/2473933002
> > 
> > Thanks yunchao for this patch, I used it to find a number of edits at
> different
> > places in the code which I had missed on my first run through.
> 
> Fantastic! Thanks Yunchao for your CL and Kai for following through on this.

That's OK. I saw the final patch at https://codereview.chromium.org/2473933002/.
A great work, Kai!