New issue
Advanced search Search tips

Issue 728790 link

Starred by 1 user
Status: Fixed
Owner:
liber...@chromium.org
Closed: Jun 2017
Cc:
boliu@chromium.org
tguilbert@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

CompositorSurfaceManager gets arg order wrong for synthetic surfaceCreated call

Project Member Reported by liber...@chromium.org, Jun 1 2017 
when sending a synthetic surfaceCreated event CSM, sends "width, height, format" instead of "format, width, height".  This greatly breaks things.

unfortunately, the tests miss it because they match anyInt() for the callback.

tguilbert uncovered this issue while adding AndroidOverlay support.  previously, synthetic create events are pretty rare, maybe impossible.

Comment 1 by liber...@chromium.org, Jun 5 2017

Status: Fixed (was: Started)
no idea why there's no commit email, but this landed the other day.  the bug is the right number, too:

https://codereview.chromium.org/2916153002/

Fix CompositorSurfaceManager synthetic surfaceCreated arg order.

When sending a synthetic surfaceChanged to the client, CSM swapped
the order of the width, height pair with the surface format.  As a
result, the compositor got the wrong value for all of them.

This CL also updates the test to check for this case, since it was
using anyInt() in the matchers previously.

BUG= 728790 
TEST=CompositorSurfaceManagerTest

Comment 2 by liber...@chromium.org, Jun 5 2017

Labels: Merge-Request-60
requesting merge just in case there's an edge case that actually hits this path.
Project Member

Comment 3 by sheriffbot@chromium.org, Jun 6 2017

Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 7 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/54644fd3cf8319f81869a3a557922b37d6ff1f27

commit 54644fd3cf8319f81869a3a557922b37d6ff1f27
Author: liberato@chromium.org <liberato@chromium.org>
Date: Wed Jun 07 20:56:01 2017

Fix CompositorSurfaceManager synthetic surfaceCreated arg order.

[Merge to M60]

When sending a synthetic surfaceChanged to the client, CSM swapped
the order of the width, height pair with the surface format.  As a
result, the compositor got the wrong value for all of them.

This CL also updates the test to check for this case, since it was
using anyInt() in the matchers previously.

BUG= 728790 
TEST=CompositorSurfaceManagerTest

Review-Url: https://codereview.chromium.org/2916153002
Review-Url: https://codereview.chromium.org/2928483006 .
Cr-Commit-Position: refs/branch-heads/3112@{#233}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/54644fd3cf8319f81869a3a557922b37d6ff1f27/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManager.java
[modify] https://crrev.com/54644fd3cf8319f81869a3a557922b37d6ff1f27/chrome/android/junit/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManagerTest.java
►

Sign in to add a comment