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

Issue 2658293002: content: Remove Lock/Unlock CompositingSurface API from RWH. (Closed)

Created:
3 years, 10 months ago by Khushal
Modified:
3 years, 9 months ago
Reviewers:
boliu, piman
CC:
chromium-reviews, creis+watch_chromium.org, yusukes+watch_chromium.org, shuchen+watch_chromium.org, nasko+codewatch_chromium.org, jam, jbauman+watch_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, piman+watch_chromium.org, kalyank, danakj+watch_chromium.org, James Su
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

content: Remove Lock/Unlock CompositingSurface API from RWH. The API was only used on android to ensure that the CompositorFrame for a tab is not evicted if a readback request is pending for it. This is now achieved by creating a readback layer instead, which is attached to the browser compositor's layer tree. This layer adds a destruction dependency on the Surface to ensure that the frame is not evicted till the layer is attached to the tree. The use of the API has already been removed. This change simply removes the now dead code. TBR=piman@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2658293002 Cr-Commit-Position: refs/heads/master@{#447914} Committed: https://chromium.googlesource.com/chromium/src/+/b01f35453ffb988cbfd4aadbf21addc52c3a6d4f

Patch Set 1 #

Patch Set 2 : mac #

Total comments: 3

Patch Set 3 : last_frame_info #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -164 lines) Patch
M content/browser/frame_host/render_widget_host_view_child_frame.h View 1 2 1 chunk +0 lines, -4 lines 0 comments Download
M content/browser/frame_host/render_widget_host_view_child_frame.cc View 1 2 1 chunk +0 lines, -8 lines 0 comments Download
M content/browser/frame_host/render_widget_host_view_guest.h View 1 chunk +0 lines, -3 lines 0 comments Download
M content/browser/frame_host/render_widget_host_view_guest.cc View 1 2 1 chunk +0 lines, -8 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.h View 1 chunk +0 lines, -2 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.cc View 1 chunk +0 lines, -10 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.h View 1 2 3 chunks +0 lines, -18 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.cc View 1 2 10 chunks +2 lines, -83 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_aura.h View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_aura.cc View 1 2 1 chunk +0 lines, -8 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_base.h View 1 chunk +0 lines, -4 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_mac.h View 1 1 chunk +0 lines, -2 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_mac.mm View 1 2 1 chunk +0 lines, -8 lines 0 comments Download
M content/public/browser/render_widget_host.h View 1 chunk +0 lines, -2 lines 0 comments Download
M content/test/test_render_view_host.h View 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 23 (16 generated)
Khushal
I was planning on also adding a test for the logic that we now depend ...
3 years, 10 months ago (2017-02-02 01:40:00 UTC) #11
Khushal
https://codereview.chromium.org/2658293002/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc (left): https://codereview.chromium.org/2658293002/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc#oldcode1981 content/browser/renderer_host/render_widget_host_view_android.cc:1981: ReleaseLocksOnSurface(); So at the other place where we were ...
3 years, 10 months ago (2017-02-02 01:46:35 UTC) #12
boliu
one more suggestion to remove code, then lgtm https://codereview.chromium.org/2658293002/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc (left): https://codereview.chromium.org/2658293002/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc#oldcode652 content/browser/renderer_host/render_widget_host_view_android.cc:652: if ...
3 years, 10 months ago (2017-02-02 22:58:48 UTC) #13
Khushal
I'm going to TBR piman for content/public and test changes. Just removes the API from ...
3 years, 10 months ago (2017-02-02 23:25:00 UTC) #15
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/2658293002/40001
3 years, 10 months ago (2017-02-02 23:26:26 UTC) #19
commit-bot: I haz the power
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/b01f35453ffb988cbfd4aadbf21addc52c3a6d4f
3 years, 10 months ago (2017-02-03 03:51:14 UTC) #22
piman
3 years, 9 months ago (2017-03-06 21:57:33 UTC) #23
Message was sent while issue was closed.
lgtm

Powered by Google App Engine
This is Rietveld 408576698