Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2856913002:
Don't submit empty frames in android webview (Closed)
|
Created:
3 years, 7 months ago by Saman Sami Modified:
3 years, 7 months ago CC:
chromium-reviews, cc-bugs_chromium.org, android-webview-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionDon't submit empty frames in android webview
cc will soon reject frames with no render passes. Submit a frame with
a solid color quad of right size instead of an empty frame in
SurfacesInstance. Also, we just get rid of the empty frame submission in
HardwareRenderer because when SurfacesInstance updates its
references_surfaces the surface of HardwareRenderer will be garbage
collected and resources will be returned.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2856913002
Cr-Commit-Position: refs/heads/master@{#469498}
Committed: https://chromium.googlesource.com/chromium/src/+/111449d5ff50dab0ea64b157d9fcca71a87c68c6
Patch Set 1 #Patch Set 2 : c #Patch Set 3 : c #Patch Set 4 : c #
Total comments: 4
Patch Set 5 : c #Patch Set 6 : c #Patch Set 7 : c #Patch Set 8 : c #
Messages
Total messages: 43 (37 generated)
Description was changed from ========== Test ========== to ========== Test CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
The CQ bit was checked by samans@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by samans@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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...)
The CQ bit was checked by samans@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by samans@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Test CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Don't submit empty frames in android webview cc will soon reject frames with no render passes. Submit a frame with a solid color quad of right size instead of an empty frame in SurfacesInstance. Also, we just get rid of the empty frame submission in HardwareRenderer because when SurfacesInstance updates its references_surfaces the surface of HardwareRenderer will be garbage collected and resources will be returned. CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
samans@chromium.org changed reviewers: + boliu@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
PTAL
https://codereview.chromium.org/2856913002/diff/60001/android_webview/browser... File android_webview/browser/surfaces_instance.cc (right): https://codereview.chromium.org/2856913002/diff/60001/android_webview/browser... android_webview/browser/surfaces_instance.cc:171: void SurfacesInstance::SetEmptyRootFrame() { rename the method name and variable names please, since it's no longer empty. should reflect that this sets a frame that does not refer to any other surface ids https://codereview.chromium.org/2856913002/diff/60001/android_webview/browser... android_webview/browser/surfaces_instance.cc:172: if (!root_id_.is_valid()) this should be a DCHECK it looks like? every call to this method already have this check
The CQ bit was checked by samans@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by samans@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
The CQ bit was checked by samans@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by samans@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
samans@chromium.org changed reviewers: + fsamuel@chromium.org
PTAL https://codereview.chromium.org/2856913002/diff/60001/android_webview/browser... File android_webview/browser/surfaces_instance.cc (right): https://codereview.chromium.org/2856913002/diff/60001/android_webview/browser... android_webview/browser/surfaces_instance.cc:171: void SurfacesInstance::SetEmptyRootFrame() { On 2017/05/04 17:08:00, boliu wrote: > rename the method name and variable names please, since it's no longer empty. > should reflect that this sets a frame that does not refer to any other surface > ids Done. https://codereview.chromium.org/2856913002/diff/60001/android_webview/browser... android_webview/browser/surfaces_instance.cc:172: if (!root_id_.is_valid()) On 2017/05/04 17:08:00, boliu wrote: > this should be a DCHECK it looks like? every call to this method already have > this check Done.
lgtm
The CQ bit was checked by samans@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 140001, "attempt_start_ts": 1493937744068790,
"parent_rev": "259bce9e315438c00d7d660b446de65b6b03c360", "commit_rev":
"111449d5ff50dab0ea64b157d9fcca71a87c68c6"}
Message was sent while issue was closed.
Description was changed from ========== Don't submit empty frames in android webview cc will soon reject frames with no render passes. Submit a frame with a solid color quad of right size instead of an empty frame in SurfacesInstance. Also, we just get rid of the empty frame submission in HardwareRenderer because when SurfacesInstance updates its references_surfaces the surface of HardwareRenderer will be garbage collected and resources will be returned. CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Don't submit empty frames in android webview cc will soon reject frames with no render passes. Submit a frame with a solid color quad of right size instead of an empty frame in SurfacesInstance. Also, we just get rid of the empty frame submission in HardwareRenderer because when SurfacesInstance updates its references_surfaces the surface of HardwareRenderer will be garbage collected and resources will be returned. CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2856913002 Cr-Commit-Position: refs/heads/master@{#469498} Committed: https://chromium.googlesource.com/chromium/src/+/111449d5ff50dab0ea64b157d9fc... ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as https://chromium.googlesource.com/chromium/src/+/111449d5ff50dab0ea64b157d9fc... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
