Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(230)

Issue 2801163002: Deletes NativeViewGLSurfaceEGL::Initialize(gfx::VSyncProvider). (Closed)

Created:
3 years, 8 months ago by alokp
Modified:
3 years, 8 months ago
CC:
chromium-reviews, alokp+watch_chromium.org, lcwu+watch_chromium.org, ozone-reviews_chromium.org, halliwell+watch_chromium.org, kalyank
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Deletes NativeViewGLSurfaceEGL::Initialize(gfx::VSyncProvider). VsyncProvider is only customized by subclasses of NativeViewGLSurfaceEGL, which can also be done using the virtual GLSurface::GetVSyncProvider member function. NativeViewGLSurfaceEGL::GetVSyncProvider now lazily creates the VSyncProvider so that subclasses that override GLSurface::GetVSyncProvider do not unnecessarily get two vsync providers. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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 Review-Url: https://codereview.chromium.org/2801163002 Cr-Commit-Position: refs/heads/master@{#463150} Committed: https://chromium.googlesource.com/chromium/src/+/dc4c6262aae691d5e6560f32ee41375be5609f29

Patch Set 1 #

Patch Set 2 : removes factory #

Patch Set 3 : passes vsync provider in constructor #

Patch Set 4 : fixes formatting #

Total comments: 1

Patch Set 5 : fixes compile errors #

Patch Set 6 : fixes compile errors on windows #

Patch Set 7 : fixes DirectCompositionSurfaceTest #

Unified diffs Side-by-side diffs Delta from patch set Stats (+64 lines, -167 lines) Patch
M chromecast/graphics/BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
D chromecast/graphics/cast_vsync_settings.h View 1 chunk +0 lines, -48 lines 0 comments Download
D chromecast/graphics/cast_vsync_settings.cc View 1 chunk +0 lines, -44 lines 0 comments Download
M chromecast/graphics/cast_window_manager_aura.h View 3 chunks +1 line, -6 lines 0 comments Download
M chromecast/graphics/cast_window_manager_aura.cc View 3 chunks +0 lines, -10 lines 0 comments Download
M gpu/ipc/service/child_window_surface_win.h View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M gpu/ipc/service/child_window_surface_win.cc View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M gpu/ipc/service/direct_composition_surface_win.h View 1 2 3 4 5 2 chunks +1 line, -2 lines 0 comments Download
M gpu/ipc/service/direct_composition_surface_win.cc View 1 2 3 4 5 2 chunks +4 lines, -7 lines 0 comments Download
M gpu/ipc/service/direct_composition_surface_win_unittest.cc View 1 2 3 4 5 6 6 chunks +7 lines, -7 lines 0 comments Download
M gpu/ipc/service/image_transport_surface_android.cc View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
M gpu/ipc/service/image_transport_surface_win.cc View 1 2 3 4 5 1 chunk +7 lines, -6 lines 0 comments Download
M ui/gl/gl_surface_egl.h View 1 2 3 4 5 3 chunks +4 lines, -6 lines 0 comments Download
M ui/gl/gl_surface_egl.cc View 1 2 3 7 chunks +14 lines, -12 lines 0 comments Download
M ui/gl/gl_surface_egl_x11.cc View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download
M ui/gl/init/gl_factory_android.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M ui/gl/init/gl_factory_win.cc View 1 2 3 4 1 chunk +2 lines, -7 lines 0 comments Download
M ui/ozone/platform/cast/gl_surface_cast.cc View 1 2 1 chunk +13 lines, -1 line 0 comments Download
M ui/ozone/platform/wayland/gl_surface_wayland.cc View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
M ui/ozone/platform/x11/x11_surface_factory.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 57 (39 generated)
halliwell
On 2017/04/06 21:40:44, commit-bot: I haz the power wrote: > Dry run: CQ is trying ...
3 years, 8 months ago (2017-04-06 21:56:18 UTC) #3
alokp
John: I will fixup the windows redness if this looks reasonable.
3 years, 8 months ago (2017-04-07 00:33:11 UTC) #13
jbauman
3 years, 8 months ago (2017-04-07 01:31:42 UTC) #15
jbauman
On 2017/04/07 00:33:11, alokp (OOO until Apr 16) wrote: > John: I will fixup the ...
3 years, 8 months ago (2017-04-07 21:31:08 UTC) #17
alokp
On 2017/04/07 21:31:08, jbauman wrote: > On 2017/04/07 00:33:11, alokp (OOO until Apr 16) wrote: ...
3 years, 8 months ago (2017-04-07 22:03:03 UTC) #18
jbauman
On 2017/04/07 22:03:03, alokp (OOO until Apr 16) wrote: > On 2017/04/07 21:31:08, jbauman wrote: ...
3 years, 8 months ago (2017-04-07 22:11:54 UTC) #19
alokp
On 2017/04/07 22:11:54, jbauman wrote: > On 2017/04/07 22:03:03, alokp (OOO until Apr 16) wrote: ...
3 years, 8 months ago (2017-04-07 22:17:30 UTC) #20
jbauman
On 2017/04/07 22:17:30, alokp (OOO until Apr 16) wrote: > On 2017/04/07 22:11:54, jbauman wrote: ...
3 years, 8 months ago (2017-04-07 22:30:50 UTC) #21
alokp
On 2017/04/07 22:30:50, jbauman wrote: > On 2017/04/07 22:17:30, alokp (OOO until Apr 16) wrote: ...
3 years, 8 months ago (2017-04-07 22:56:45 UTC) #22
jbauman
On 2017/04/07 22:56:45, alokp (OOO until Apr 16) wrote: > On 2017/04/07 22:30:50, jbauman wrote: ...
3 years, 8 months ago (2017-04-07 22:59:20 UTC) #23
alokp
The latest patch passes vsync provider in constructor. Please review gl_surface_egl.[h,cc]. I will fix the ...
3 years, 8 months ago (2017-04-07 23:25:54 UTC) #24
jbauman
lgtm https://codereview.chromium.org/2801163002/diff/60001/ui/gl/gl_surface_egl.cc File ui/gl/gl_surface_egl.cc (right): https://codereview.chromium.org/2801163002/diff/60001/ui/gl/gl_surface_egl.cc#newcode812 ui/gl/gl_surface_egl.cc:812: if (!vsync_provider_external_ && EGLSyncControlVSyncProvider::IsSupported()) { DCHECK(!vsync_provider_internal_);
3 years, 8 months ago (2017-04-07 23:29:44 UTC) #27
alokp
avi: ui/ozone/platform/[wayland,x11]
3 years, 8 months ago (2017-04-08 04:48:49 UTC) #34
Avi (use Gerrit)
On 2017/04/08 04:48:49, alokp (OOO until Apr 16) wrote: > avi: ui/ozone/platform/[wayland,x11] Those files lgtm ...
3 years, 8 months ago (2017-04-08 05:04:40 UTC) #37
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2801163002/120001
3 years, 8 months ago (2017-04-09 02:54:25 UTC) #50
commit-bot: I haz the power
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_android_rel_ng/builds/267824)
3 years, 8 months ago (2017-04-09 06:05:59 UTC) #52
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2801163002/120001
3 years, 8 months ago (2017-04-09 08:30:56 UTC) #54
commit-bot: I haz the power
3 years, 8 months ago (2017-04-09 21:55:06 UTC) #57
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/dc4c6262aae691d5e6560f32ee41...

Powered by Google App Engine
This is Rietveld 408576698