|
|
Created:
4 years, 9 months ago by GeorgeZ Modified:
4 years, 9 months ago CC:
chromium-reviews, feature-media-reviews_chromium.org, mcasas+watch_chromium.org, posciak+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix NativeDesktopMediaList to set aura_id for chrome Aura windows for Linux and windows.
Previously NativeDesktopMediaList was returning only native window ID, now it also sets aura_id for chrome in Linux and windows. This allows to capture browser windows directly from Aura, which is faster and more reliable.
BUG=581790, 289779, 590915
Committed: https://crrev.com/a21a35a14f00f2a14681f50454b7eff79fda7765
Cr-Commit-Position: refs/heads/master@{#381711}
Patch Set 1 : #
Total comments: 14
Patch Set 2 : #
Total comments: 24
Patch Set 3 : #
Total comments: 8
Patch Set 4 : #
Total comments: 4
Patch Set 5 : #
Messages
Total messages: 40 (17 generated)
Message was sent while issue was closed.
Description was changed from ========== Capture chrome browser windows from internal rendering procedure instead of based on the operating system. BUG=581790, 289779 ========== to ========== Capture chrome browser windows from internal rendering procedure instead of based on the operating system. BUG=581790, 289779 ==========
Description was changed from ========== Capture chrome browser windows from internal rendering procedure instead of based on the operating system. BUG=581790, 289779 ========== to ========== Capture chrome browser windows from internal rendering procedure instead of based on the operating system. BUG=581790, 289779 ==========
Patchset #1 (id:1) has been deleted
Description was changed from ========== Capture chrome browser windows from internal rendering procedure instead of based on the operating system. BUG=581790, 289779 ========== to ========== Capture chrome browser windows from internal rendering procedure instead of based on the operating system. BUG=581790, 289779, 590915 ==========
Patchset #2 (id:40001) has been deleted
Patchset #1 (id:20001) has been deleted
gyzhou@chromium.org changed reviewers: + jiayl@chromium.org, sergeyu@chromium.org
Sergey and Jiayang, Could you have a look for this CL. Jiayang, You may just focus on content/browser/renderer_host/media/media_stream_manager.cc. After your review, I will find a owner of the file for approve. Thanks,
https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... File chrome/browser/media/native_desktop_media_list.cc (right): https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:192: if (source.id.aura_id > 0) Add a comment to explain what happens here. https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:192: if (source.id.aura_id > 0) aura_id != DesktopMediaID::kInvalidId. https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:238: if (aura_media_ids.size() == 0) { The logic here is hard to follow. I suggest you post a single task to UI thread with all aura_media_ids and let it schedule next refresh. I.e. add FinishRefreshOnUiThreadAndScheduleNext() which will snapshot all aura window and call ScheduleNextRefresh() https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:342: if (window == NULL) { if (!window) for consistency with the code above https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:357: #endif // defined(USE_AURA) https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... File chrome/browser/media/native_desktop_media_list.h (right): https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.h:60: ImageHashesMap cur_aura_thumbnail_hashes_; call this new_aura_thumbnail_hashes_? Also add comment to explain what they are used for and why you need 2 maps here. https://codereview.chromium.org/1763753003/diff/60001/content/browser/rendere... File content/browser/renderer_host/media/media_stream_manager.cc (right): https://codereview.chromium.org/1763753003/diff/60001/content/browser/rendere... content/browser/renderer_host/media/media_stream_manager.cc:2056: // Pass along for desktop screen and window capturing when DesktopCaptureDevic typo: DesktopCaptureDevic
Sergey, I have update based on your comments. Thanks, https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... File chrome/browser/media/native_desktop_media_list.cc (right): https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:192: if (source.id.aura_id > 0) On 2016/03/07 21:01:50, Sergey Ulanov wrote: > Add a comment to explain what happens here. Done. https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:192: if (source.id.aura_id > 0) On 2016/03/07 21:01:50, Sergey Ulanov wrote: > aura_id != DesktopMediaID::kInvalidId. Done. https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:238: if (aura_media_ids.size() == 0) { On 2016/03/07 21:01:50, Sergey Ulanov wrote: > The logic here is hard to follow. I suggest you post a single task to UI thread > with all aura_media_ids and let it schedule next refresh. > I.e. add FinishRefreshOnUiThreadAndScheduleNext() which will snapshot all aura > window and call ScheduleNextRefresh() Good idea. https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:342: if (window == NULL) { On 2016/03/07 21:01:50, Sergey Ulanov wrote: > if (!window) for consistency with the code above Done. https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:357: #endif On 2016/03/07 21:01:50, Sergey Ulanov wrote: > // defined(USE_AURA) Done. https://codereview.chromium.org/1763753003/diff/60001/content/browser/rendere... File content/browser/renderer_host/media/media_stream_manager.cc (right): https://codereview.chromium.org/1763753003/diff/60001/content/browser/rendere... content/browser/renderer_host/media/media_stream_manager.cc:2056: // Pass along for desktop screen and window capturing when DesktopCaptureDevic On 2016/03/07 21:01:51, Sergey Ulanov wrote: > typo: DesktopCaptureDevic Done.
Ping Sergey and JiaYang, https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... File chrome/browser/media/native_desktop_media_list.h (right): https://codereview.chromium.org/1763753003/diff/60001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.h:60: ImageHashesMap cur_aura_thumbnail_hashes_; On 2016/03/07 21:01:50, Sergey Ulanov wrote: > call this new_aura_thumbnail_hashes_? > Also add comment to explain what they are used for and why you need 2 maps here. Done.
Sergey's review will be sufficient. Let me know if you just need a rubber stamp.
https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/de... File chrome/browser/media/desktop_media_list_base.h (right): https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/de... chrome/browser/media/desktop_media_list_base.h:12: using content::DesktopMediaID; using is not allowed in headers. Please use fully qualified name https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... File chrome/browser/media/native_desktop_media_list.cc (right): https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:161: media_id.aura_id = native_aura_id_map[media_id.id]; nit: here you search the map twice, which can be avoided: auto aura_id = native_aura_id_map.find(media_id.id); if (aura_id != native_aura_id_map.end()) { media_id.aura_id = aura_id->second; aura_media_ids.push_back(aura_id); } https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:192: // Aura window thumbmail capture is skipped here. It will be done later. maybe also mention the UI thread. Otherwise it's not clear why it needs to be skipped. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:281: std::vector<DesktopMediaID> aura_ids) { use const reference for the parameter https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:285: total_aura_windows_ = aura_ids.size(); Instead of having both processed_aura_window_count_ and total_aura_windows_ you can have a single counter for pending Aura captures and decrements it in OnAuraThumbnailCaptured(). https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:285: total_aura_windows_ = aura_ids.size(); Add a DCHECK here to verify there are no pending capture requests, e.g. DCHECK(pending_aura_capture_requests_ == 0); https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:310: swap(previous_aura_thumbnail_hashes_, new_aura_thumbnail_hashes_); nit: previous_aura_thumbnail_hashes_ = std::move(new_aura_thumbnail_hashes_); https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:318: NativeDesktopMediaList::GetBrowserNativeAuraIdMap() { This function doesn't need to be a class member. Make it static function in .cc file. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:321: const BrowserWindow* browser_window = browser->window(); nit: I don't think you need this variable. Just use browser_window() below? https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:324: const gfx::NativeWindow native_window = browser_window->GetNativeWindow(); Suggest changing this to: aura::Window* aura_window = browser_window->GetNativeWindow(); Otherwise this code is rather confusing. NativeWindow is a synonym for aura::Window* with USE_AURA. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:338: id_map.insert(std::make_pair(native_id, media_id.aura_id)); nit: id_map[native_id] = media_id.aura_id; That would be more readable. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/ta... File chrome/browser/media/tab_desktop_media_list.cc (right): https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/ta... chrome/browser/media/tab_desktop_media_list.cc:138: DesktopMediaListBase::GetImageHash(favicon); I don't think you need DesktopMediaListBase:: here. This class inherits from DesktopMediaListBase.
Also I think this CL deserves a better description. Specifically it's not clear that it only affects Aura-based build (i.e. doesn't change anything on OSX). E.g. I suggest the following: """ Fix NativeDesktopMediaList to set aura_id for Aura windows. Previously NativeDesktopMediaList was returning only native window ID, now it also sets aura_id. This allows to capture browser windows directly from Aura, which is faster and more reliable. """
Description was changed from ========== Capture chrome browser windows from internal rendering procedure instead of based on the operating system. BUG=581790, 289779, 590915 ========== to ========== Fix NativeDesktopMediaList to set aura_id for chrome Aura windows for Linux and windows. Previously NativeDesktopMediaList was returning only native window ID, now it also sets aura_id for chrome in Linux and windows. This allows to capture browser windows directly from Aura, which is faster and more reliable. BUG=581790, 289779, 590915 ==========
Patchset #3 (id:100001) has been deleted
Patchset #3 (id:120001) has been deleted
Sergey, I updated code based your comments. Thanks https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/de... File chrome/browser/media/desktop_media_list_base.h (right): https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/de... chrome/browser/media/desktop_media_list_base.h:12: using content::DesktopMediaID; On 2016/03/10 00:46:07, Sergey Ulanov wrote: > using is not allowed in headers. Please use fully qualified name Done. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... File chrome/browser/media/native_desktop_media_list.cc (right): https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:161: media_id.aura_id = native_aura_id_map[media_id.id]; On 2016/03/10 00:46:07, Sergey Ulanov wrote: > nit: here you search the map twice, which can be avoided: > auto aura_id = native_aura_id_map.find(media_id.id); > if (aura_id != native_aura_id_map.end()) { > media_id.aura_id = aura_id->second; > aura_media_ids.push_back(aura_id); > } good point. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:192: // Aura window thumbmail capture is skipped here. It will be done later. On 2016/03/10 00:46:07, Sergey Ulanov wrote: > maybe also mention the UI thread. Otherwise it's not clear why it needs to be > skipped. I will also mention it will be done asynchronously. This is the major reason that it need be skipped here. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:281: std::vector<DesktopMediaID> aura_ids) { On 2016/03/10 00:46:07, Sergey Ulanov wrote: > use const reference for the parameter Done. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:285: total_aura_windows_ = aura_ids.size(); On 2016/03/10 00:46:07, Sergey Ulanov wrote: > Instead of having both processed_aura_window_count_ and total_aura_windows_ you > can have a single counter for pending Aura captures and decrements it in > OnAuraThumbnailCaptured(). Done. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:285: total_aura_windows_ = aura_ids.size(); On 2016/03/10 00:46:07, Sergey Ulanov wrote: > Add a DCHECK here to verify there are no pending capture requests, e.g. > DCHECK(pending_aura_capture_requests_ == 0); Done. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:310: swap(previous_aura_thumbnail_hashes_, new_aura_thumbnail_hashes_); On 2016/03/10 00:46:07, Sergey Ulanov wrote: > nit: > previous_aura_thumbnail_hashes_ = std::move(new_aura_thumbnail_hashes_); good point. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:318: NativeDesktopMediaList::GetBrowserNativeAuraIdMap() { On 2016/03/10 00:46:07, Sergey Ulanov wrote: > This function doesn't need to be a class member. Make it static function in .cc > file. Done. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:321: const BrowserWindow* browser_window = browser->window(); On 2016/03/10 00:46:07, Sergey Ulanov wrote: > nit: I don't think you need this variable. Just use browser_window() below? Done. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:324: const gfx::NativeWindow native_window = browser_window->GetNativeWindow(); On 2016/03/10 00:46:08, Sergey Ulanov wrote: > Suggest changing this to: > aura::Window* aura_window = browser_window->GetNativeWindow(); > Otherwise this code is rather confusing. NativeWindow is a synonym for > aura::Window* with USE_AURA. Done. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/na... chrome/browser/media/native_desktop_media_list.cc:338: id_map.insert(std::make_pair(native_id, media_id.aura_id)); On 2016/03/10 00:46:07, Sergey Ulanov wrote: > nit: id_map[native_id] = media_id.aura_id; > That would be more readable. Done. https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/ta... File chrome/browser/media/tab_desktop_media_list.cc (right): https://codereview.chromium.org/1763753003/diff/80001/chrome/browser/media/ta... chrome/browser/media/tab_desktop_media_list.cc:138: DesktopMediaListBase::GetImageHash(favicon); On 2016/03/10 00:46:08, Sergey Ulanov wrote: > I don't think you need DesktopMediaListBase:: here. This class inherits from > DesktopMediaListBase. Done.
gyzhou@chromium.org changed reviewers: + dalecurtis@chromium.org
dalecurtis@ As owner, could you have a look of content/browser/renderer_host/media/media_stream_manager.cc. I consulted with the auhtor (jiayl@) of code that I had changed in that file. Thanks,
I'll stamp after jiayl@ and sergeyu@ have approved.
Is it possible to add unittests for the new code? https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/d... File chrome/browser/media/desktop_media_list_base.cc (right): https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/d... chrome/browser/media/desktop_media_list_base.cc:136: // Static nit: lowercase s https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/d... File chrome/browser/media/desktop_media_list_base.h (right): https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/d... chrome/browser/media/desktop_media_list_base.h:10: #include "ui/gfx/image/image.h" forward-declare gfx::Image instead of including this file. https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/n... File chrome/browser/media/native_desktop_media_list.cc (right): https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/n... chrome/browser/media/native_desktop_media_list.cc:321: DCHECK(pending_aura_capture_requests_ == 0); nit: DCHECK_EQ https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/n... chrome/browser/media/native_desktop_media_list.cc:358: pending_aura_capture_requests_--; DCHECK_GT(pending_aura_capture_requests_, 0)
bonillaglenda0@gmail.com changed reviewers: + bonillaglenda0@gmail.com
lgtm
Sergey, I should be able to add unit tests for newly added code. From my experience for tab unit tests, it will not be a trivial task. Also I would like to investigate whether I can cover all aura windows instead of chromes. If it is possible, the associated unit test may be somewhat different. Therefore I would like to put the additional unit test in a separated CL. Other of your comments are addressed in new patch. Thanks, https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/d... File chrome/browser/media/desktop_media_list_base.cc (right): https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/d... chrome/browser/media/desktop_media_list_base.cc:136: // Static On 2016/03/12 01:01:20, Sergey Ulanov wrote: > nit: lowercase s Done. https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/d... File chrome/browser/media/desktop_media_list_base.h (right): https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/d... chrome/browser/media/desktop_media_list_base.h:10: #include "ui/gfx/image/image.h" On 2016/03/12 01:01:20, Sergey Ulanov wrote: > forward-declare gfx::Image instead of including this file. Done. https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/n... File chrome/browser/media/native_desktop_media_list.cc (right): https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/n... chrome/browser/media/native_desktop_media_list.cc:321: DCHECK(pending_aura_capture_requests_ == 0); On 2016/03/12 01:01:20, Sergey Ulanov wrote: > nit: DCHECK_EQ Done. https://codereview.chromium.org/1763753003/diff/140001/chrome/browser/media/n... chrome/browser/media/native_desktop_media_list.cc:358: pending_aura_capture_requests_--; On 2016/03/12 01:01:20, Sergey Ulanov wrote: > DCHECK_GT(pending_aura_capture_requests_, 0) Done.
lgtm
JiaYang and DaleCurtis, Could you approve this CL after Sergey approved. Thanks,
lgtm
DaleCurtis, Could you approve this CL since Sergey and jiayl had approved? Thanks,
https://codereview.chromium.org/1763753003/diff/160001/chrome/browser/media/n... File chrome/browser/media/native_desktop_media_list.cc (right): https://codereview.chromium.org/1763753003/diff/160001/chrome/browser/media/n... chrome/browser/media/native_desktop_media_list.cc:116: NativeAuraIdMap native_aura_id_map); This appears to be passing by copy, is that intentional? https://codereview.chromium.org/1763753003/diff/160001/chrome/browser/media/n... chrome/browser/media/native_desktop_media_list.cc:154: NativeAuraIdMap native_aura_id_map) { Ditto.
Patchset #5 (id:180001) has been deleted
Dalecurtis, I uploaded a patch based on your comment. Thanks, https://codereview.chromium.org/1763753003/diff/160001/chrome/browser/media/n... File chrome/browser/media/native_desktop_media_list.cc (right): https://codereview.chromium.org/1763753003/diff/160001/chrome/browser/media/n... chrome/browser/media/native_desktop_media_list.cc:116: NativeAuraIdMap native_aura_id_map); On 2016/03/16 21:10:00, DaleCurtis wrote: > This appears to be passing by copy, is that intentional? Can be constant. Good catch. https://codereview.chromium.org/1763753003/diff/160001/chrome/browser/media/n... chrome/browser/media/native_desktop_media_list.cc:154: NativeAuraIdMap native_aura_id_map) { On 2016/03/16 21:10:00, DaleCurtis wrote: > Ditto. Done.
lgtm
The CQ bit was checked by gyzhou@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bonillaglenda0@gmail.com, sergeyu@chromium.org, jiayl@chromium.org Link to the patchset: https://codereview.chromium.org/1763753003/#ps200001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1763753003/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1763753003/200001
Message was sent while issue was closed.
Description was changed from ========== Fix NativeDesktopMediaList to set aura_id for chrome Aura windows for Linux and windows. Previously NativeDesktopMediaList was returning only native window ID, now it also sets aura_id for chrome in Linux and windows. This allows to capture browser windows directly from Aura, which is faster and more reliable. BUG=581790, 289779, 590915 ========== to ========== Fix NativeDesktopMediaList to set aura_id for chrome Aura windows for Linux and windows. Previously NativeDesktopMediaList was returning only native window ID, now it also sets aura_id for chrome in Linux and windows. This allows to capture browser windows directly from Aura, which is faster and more reliable. BUG=581790, 289779, 590915 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:200001)
Message was sent while issue was closed.
Description was changed from ========== Fix NativeDesktopMediaList to set aura_id for chrome Aura windows for Linux and windows. Previously NativeDesktopMediaList was returning only native window ID, now it also sets aura_id for chrome in Linux and windows. This allows to capture browser windows directly from Aura, which is faster and more reliable. BUG=581790, 289779, 590915 ========== to ========== Fix NativeDesktopMediaList to set aura_id for chrome Aura windows for Linux and windows. Previously NativeDesktopMediaList was returning only native window ID, now it also sets aura_id for chrome in Linux and windows. This allows to capture browser windows directly from Aura, which is faster and more reliable. BUG=581790, 289779, 590915 Committed: https://crrev.com/a21a35a14f00f2a14681f50454b7eff79fda7765 Cr-Commit-Position: refs/heads/master@{#381711} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/a21a35a14f00f2a14681f50454b7eff79fda7765 Cr-Commit-Position: refs/heads/master@{#381711} |