ozone/wayland: Initialize WaylandBufferManagerGpu with bug fix ids [chromium/src : main]
Eriko Kurimoto (Gerrit)
Attention is currently required from: Eliot Courtney, Eriko Kurimoto, Mitsuru Oshima.
Eriko Kurimoto uploaded patch set #8 to this change.
ozone/wayland: Initialize WaylandBufferManagerGpu with bug fix ids
This CL lets WaylandBufferManagerConenctor initialize
WaylandBufferManagerGpu with bug fix ids.
Bug fix ids are sent from the server asynchronously and stored in
zaura_shell, so it needs to wait until all bug fix ids are sent. We can confirm all bug fix ids are sent by receiving `bug_fix_done` event.
Bug: 1480226
Change-Id: Ie525d7912f3a47cd1278fcd5379b03122cb798f3
---
M ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.cc
M ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.h
M ui/ozone/platform/wayland/gpu/wayland_overlay_manager_unittest.cc
M ui/ozone/platform/wayland/gpu/wayland_surface_factory_unittest.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_connector.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_connector.h
M ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h
M ui/ozone/platform/wayland/host/wayland_window_unittest.cc
M ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
M ui/ozone/platform/wayland/host/wayland_zaura_shell.h
M ui/ozone/platform/wayland/host/wayland_zaura_shell_unittest.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager_unittest.cc
M ui/ozone/platform/wayland/mojom/wayland_buffer_manager.mojom
M ui/ozone/platform/wayland/test/test_zaura_shell.cc
M ui/ozone/platform/wayland/test/test_zaura_shell.h
M ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc
17 files changed, 229 insertions(+), 32 deletions(-)
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Eliot Courtney, Eriko Kurimoto, Mitsuru Oshima.
Eriko Kurimoto uploaded patch set #9 to this change.
The following approvals got outdated and were removed: Commit-Queue+1 by Eriko Kurimoto
Eriko Kurimoto (Gerrit)
Attention is currently required from: Eliot Courtney, Mitsuru Oshima.
Patch set 9:Commit-Queue +1
1 comment:
Patchset:
Seperated a cl: sever side impl -> https://chromium-review.googlesource.com/c/chromium/src/+/4857671
Added comments.PTAL.
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Eliot Courtney, Eriko Kurimoto, Mitsuru Oshima.
Eriko Kurimoto uploaded patch set #11 to this change.
The following approvals got outdated and were removed: Commit-Queue+1 by Eriko Kurimoto
ozone/wayland: Initialize WaylandBufferManagerGpu with bug fix ids
This CL lets WaylandBufferManagerConnector initialize
WaylandBufferManagerGpu with bug fix ids.
Bug fix ids are sent from the server asynchronously and stored in
zaura_shell, so it needs to wait until all bug fix ids are sent. We can confirm all bug fix ids are sent by receiving `bug_fix_done` event.
Bug: 1480226
Change-Id: Ie525d7912f3a47cd1278fcd5379b03122cb798f3
---
M ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.cc
M ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.h
M ui/ozone/platform/wayland/gpu/wayland_overlay_manager_unittest.cc
M ui/ozone/platform/wayland/gpu/wayland_surface_factory_unittest.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_connector.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_connector.h
M ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h
M ui/ozone/platform/wayland/host/wayland_window_unittest.cc
M ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
M ui/ozone/platform/wayland/host/wayland_zaura_shell.h
M ui/ozone/platform/wayland/host/wayland_zaura_shell_unittest.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager_unittest.cc
M ui/ozone/platform/wayland/mojom/wayland_buffer_manager.mojom
M ui/ozone/platform/wayland/test/test_zaura_shell.cc
M ui/ozone/platform/wayland/test/test_zaura_shell.h
M ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc
17 files changed, 227 insertions(+), 32 deletions(-)
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eliot Courtney (Gerrit)
Attention is currently required from: Eriko Kurimoto, Mitsuru Oshima.
5 comments:
File ui/ozone/platform/wayland/gpu/wayland_surface_factory_unittest.cc:
Patch Set #10, Line 202:
kBugFixIdin general nice to give a descriptive name for bug constants, like kBugFreezeTimer or something like that
File ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h:
Patch Set #11, Line 71:
WaitBugFixIdsnit: WaitForAllBugFixIds
Patch Set #11, Line 219:
bug_fix_callback_nit: all_bug_fixes_sent_callback_
File ui/ozone/platform/wayland/host/wayland_zaura_shell.cc:
Patch Set #11, Line 29:
std::vector<uint32_t> ConvertFlatSetIntoVector(nit: can you just call vector with begin(), end()?
File ui/ozone/platform/wayland/test/test_zaura_shell.h:
Patch Set #11, Line 33:
MaybeSendAllBugFixesSentnit: I think we don't need 'maybe' here, since it's not deciding based on current state, it's just about protocol version.
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Eriko Kurimoto, Mitsuru Oshima.
Eriko Kurimoto uploaded patch set #12 to this change.
ozone/wayland: Initialize WaylandBufferManagerGpu with bug fix ids
17 files changed, 232 insertions(+), 37 deletions(-)
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Eliot Courtney, Mitsuru Oshima.
Patch set 12:Commit-Queue +1
5 comments:
File ui/ozone/platform/wayland/gpu/wayland_surface_factory_unittest.cc:
Patch Set #10, Line 202:
kBugFixId
in general nice to give a descriptive name for bug constants, like kBugFreezeTimer or something like […]
Done
File ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h:
Patch Set #11, Line 71:
WaitBugFixIdsnit: WaitForAllBugFixIds
Done
Patch Set #11, Line 219:
bug_fix_callback_nit: all_bug_fixes_sent_callback_
Done
File ui/ozone/platform/wayland/host/wayland_zaura_shell.cc:
Patch Set #11, Line 29:
std::vector<uint32_t> ConvertFlatSetIntoVector(nit: can you just call vector with begin(), end()?
Done
File ui/ozone/platform/wayland/test/test_zaura_shell.h:
Patch Set #11, Line 33:
MaybeSendAllBugFixesSent
nit: I think we don't need 'maybe' here, since it's not deciding based on current state, it's just a […]
Updating MaybeSendBugFixes naming as well.
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eliot Courtney (Gerrit)
Attention is currently required from: Eriko Kurimoto, Mitsuru Oshima.
3 comments:
Commit Message:
Patch Set #12, Line 13:
zaura_shell, so it needs to wait until all bug fix ids are sent. We can confirm all bug fix ids are sent by receiving `bug_fix_done` event.nit: fix wrapping
File ui/ozone/platform/wayland/gpu/wayland_surface_factory_unittest.cc:
Patch Set #12, Line 202:
kFreezeTimerBugIdnit: unless there is already a convention otherwise, it's probably better to say kBugId* since then all the bug id constants will sort next to eachother + be easier to read (IMO)
File ui/ozone/platform/wayland/test/test_zaura_shell.h:
Patch Set #11, Line 33:
MaybeSendAllBugFixesSent
Updating MaybeSendBugFixes naming as well.
I see there's another function called MaybeSendCompositorVersion, it would have been okay to push back based on the file convention IMO.
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eliot Courtney (Gerrit)
Attention is currently required from: Eriko Kurimoto, Mitsuru Oshima.
Patch set 12:Code-Review +1
1 comment:
Patchset:
please wait for review from oshima@, plus you are missing some owners
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Chromium IPC Reviews, Mitsuru Oshima.
Eriko Kurimoto would like Chromium IPC Reviews to review this change.
ozone/wayland: Initialize WaylandBufferManagerGpu with bug fix ids
This CL lets WaylandBufferManagerConnector initialize
WaylandBufferManagerGpu with bug fix ids.
Bug fix ids are sent from the server asynchronously and stored in
zaura_shell, so it needs to wait until all bug fix ids are sent. We can confirm all bug fix ids are sent by receiving `bug_fix_done` event.
Bug: 1480226
Change-Id: Ie525d7912f3a47cd1278fcd5379b03122cb798f3
---
M ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.cc
M ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.h
M ui/ozone/platform/wayland/gpu/wayland_overlay_manager_unittest.cc
M ui/ozone/platform/wayland/gpu/wayland_surface_factory_unittest.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_connector.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_connector.h
M ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h
M ui/ozone/platform/wayland/host/wayland_window_unittest.cc
M ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
M ui/ozone/platform/wayland/host/wayland_zaura_shell.h
M ui/ozone/platform/wayland/host/wayland_zaura_shell_unittest.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager_unittest.cc
M ui/ozone/platform/wayland/mojom/wayland_buffer_manager.mojom
M ui/ozone/platform/wayland/test/test_zaura_shell.cc
M ui/ozone/platform/wayland/test/test_zaura_shell.h
M ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc
17 files changed, 232 insertions(+), 37 deletions(-)
gwsq (Gerrit)
Attention is currently required from: Chromium IPC Reviews, Giovanni Ortuno Urquidi, Mitsuru Oshima.
gwsq would like Giovanni Ortuno Urquidi to review this change authored by Eriko Kurimoto.
gwsq (Gerrit)
Attention is currently required from: Giovanni Ortuno Urquidi, Mitsuru Oshima.
Eriko Kurimoto has uploaded this change for review.
gwsq (Gerrit)
Attention is currently required from: Giovanni Ortuno Urquidi, Mitsuru Oshima.
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: ort...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): ort...@chromium.org
Reviewer source(s):
ort...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Giovanni Ortuno Urquidi (Gerrit)
Attention is currently required from: Eriko Kurimoto, Mitsuru Oshima.
2 comments:
Patchset:
Looks reasonable but I'll wait for oshima to +1.
File ui/ozone/platform/wayland/mojom/wayland_buffer_manager.mojom:
Patch Set #12, Line 100:
interface WaylandBufferManagerGpu {IIUC, this interface is implemented by the GPU and used by the browser. Is that correct? If so, could we add a comment to clarify?
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Eriko Kurimoto, Mitsuru Oshima.
Eriko Kurimoto uploaded patch set #13 to this change.
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Eriko Kurimoto, Mitsuru Oshima.
Eriko Kurimoto uploaded patch set #14 to this change.
The following approvals got outdated and were removed: Commit-Queue+1 by Eriko Kurimoto
ozone/wayland: Initialize WaylandBufferManagerGpu with bug fix ids
17 files changed, 234 insertions(+), 40 deletions(-)
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Giovanni Ortuno Urquidi, Mitsuru Oshima.
Patch set 14:Commit-Queue +1
3 comments:
Commit Message:
Patch Set #12, Line 13:
zaura_shell, so it needs to wait until all bug fix ids are sent. We can confirm all bug fix ids are sent by receiving `bug_fix_done` event.nit: fix wrapping
Done
File ui/ozone/platform/wayland/gpu/wayland_surface_factory_unittest.cc:
Patch Set #12, Line 202:
kFreezeTimerBugId
nit: unless there is already a convention otherwise, it's probably better to say kBugId* since then […]
Done
File ui/ozone/platform/wayland/mojom/wayland_buffer_manager.mojom:
Patch Set #12, Line 100:
interface WaylandBufferManagerGpu {
this interface is implemented by the GPU and used by the browser. Is that correct?
Yes, it's correct.
Added a comment for clarification.
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Mitsuru Oshima (Gerrit)
Attention is currently required from: Eriko Kurimoto, Giovanni Ortuno Urquidi.
Patch set 14:Code-Review +1
2 comments:
File ui/ozone/platform/wayland/host/wayland_zaura_shell.cc:
Patch Set #14, Line 124:
bug_fix_ids_.end());bug_fix_ids_ should probably be just vector to avoid conversions. (flat set is vector internally anyway)
File ui/ozone/platform/wayland/test/test_zaura_shell.cc:
Patch Set #14, Line 105:
ZAURA_SHELL_ALL_BUG_FIXES_SENT_SINCE_VERSION) {is this version check necessary?
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Eriko Kurimoto, Giovanni Ortuno Urquidi.
Eriko Kurimoto uploaded patch set #15 to this change.
ozone/wayland: Initialize WaylandBufferManagerGpu with bug fix ids
This CL lets WaylandBufferManagerConnector initialize
WaylandBufferManagerGpu with bug fix ids.
Bug fix ids are sent from the server asynchronously and stored in
zaura_shell, so it needs to wait until all bug fix ids are sent. We can
confirm all bug fix ids are sent by receiving `bug_fix_done` event.
Bug: 1480226
Change-Id: Ie525d7912f3a47cd1278fcd5379b03122cb798f3
---
M ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.cc
M ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.h
M ui/ozone/platform/wayland/gpu/wayland_overlay_manager_unittest.cc
M ui/ozone/platform/wayland/gpu/wayland_surface_factory_unittest.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_connector.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_connector.h
M ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc
M ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h
M ui/ozone/platform/wayland/host/wayland_window_unittest.cc
M ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
M ui/ozone/platform/wayland/host/wayland_zaura_shell.h
M ui/ozone/platform/wayland/host/wayland_zaura_shell_unittest.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager_unittest.cc
M ui/ozone/platform/wayland/mojom/wayland_buffer_manager.mojom
M ui/ozone/platform/wayland/test/test_zaura_shell.cc
M ui/ozone/platform/wayland/test/test_zaura_shell.h
M ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc
17 files changed, 233 insertions(+), 44 deletions(-)
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Giovanni Ortuno Urquidi, Mitsuru Oshima.
Looks reasonable but I'll wait for oshima to +1.
Oshima +1ed. PTAL again.
File ui/ozone/platform/wayland/host/wayland_zaura_shell.cc:
Patch Set #14, Line 124:
bug_fix_ids_.end());
bug_fix_ids_ should probably be just vector to avoid conversions. […]
Agree, changing it to vector.
(In theory, HasBugFix uses lookup and it's faster on flat_set, but we can assume the bug fix ids are small so it's fine).
File ui/ozone/platform/wayland/test/test_zaura_shell.cc:
Patch Set #14, Line 105:
ZAURA_SHELL_ALL_BUG_FIXES_SENT_SINCE_VERSION) {is this version check necessary?
GlobalObject::Bind may set a version, so it's needed. (I also added it in OnBind call)
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eliot Courtney (Gerrit)
Attention is currently required from: Eriko Kurimoto, Giovanni Ortuno Urquidi, Mitsuru Oshima.
Patch set 15:Code-Review +1
2 comments:
Patchset:
slgtm modulo comments
File ui/ozone/platform/wayland/host/wayland_zaura_shell.cc:
Patch Set #14, Line 124:
bug_fix_ids_.end());
Agree, changing it to vector. […]
flat_set is implemented using a vector, so this makes no difference. flat_set lookup is a linear scan, just like vector. the reason to use flat_set over vector is for semantic reasons + guaranteeing uniqueness
in general, use vector
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eliot Courtney (Gerrit)
Attention is currently required from: Eriko Kurimoto, Giovanni Ortuno Urquidi, Mitsuru Oshima.
Patch Set #14, Line 124:
bug_fix_ids_.end());
flat_set is implemented using a vector, so this makes no difference. […]
quick correction, I checked and chromium's flat_set uses binary search for find ( think I was thinking of small_map). for this size of data it will be slower than linear scan most likely
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Giovanni Ortuno Urquidi, Mitsuru Oshima.
1 comment:
File ui/ozone/platform/wayland/host/wayland_zaura_shell.cc:
Patch Set #14, Line 124:
bug_fix_ids_.end());flat_set is implemented using a vector, so this makes no difference. […]
flat_set stored data as a sorted vector, so theoritically faster, but does not mean much on this length and also it's sorted anyway in this usage.
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Giovanni Ortuno Urquidi, Mitsuru Oshima.
1 comment:
File ui/ozone/platform/wayland/host/wayland_zaura_shell.cc:
Patch Set #14, Line 124:
bug_fix_ids_.end());
ops, sorry cross comments.
for this size of data it will be slower than linear scan most likely
That's true. std::vector sounds better.
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.
Eriko Kurimoto (Gerrit)
Attention is currently required from: Eriko Kurimoto, Giovanni Ortuno Urquidi, Mitsuru Oshima.
Eriko Kurimoto uploaded patch set #16 to this change.
17 files changed, 232 insertions(+), 44 deletions(-)
To view, visit change 4856179. To unsubscribe, or for help writing mail filters, visit settings.