android: Fix black screen at startng frame when using ContentViewRenderView

android: Fix black screen at startng frame when using ContentViewRenderView

A recent change to avoid rendering an empty surface texture
(https://codereview.chromium.org/62333022) caused a black screen to appear for
users for ContentViewRenderView at the first rendering frame.  The old default
color is white.  This patch changes that first frame from black to white.

BUG=323450
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238851

[powei, 2013-11-27 20:52:25]

PTAL.  Thanks.

igsolla@ can you make sure that this does not undo the effect that you wanted
with your patch?

[no sievers, 2013-11-27 21:22:20]

Nice catch.

https://codereview.chromium.org/92603002/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
(right):

https://codereview.chromium.org/92603002/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java:63:
setBackgroundColor(Color.WHITE);
I think this can go.

https://codereview.chromium.org/92603002/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java:288:
mSurfaceView.setBackground(null);
This is only available in JB and up.

Does mSurfaceView.setBackgroundResource(0) do the trick.

And then we can remove 287 also.

[Ignacio Solla, 2013-11-28 10:19:07]

On 2013/11/27 20:52:25, powei wrote:
> PTAL.  Thanks.
> 
> igsolla@ can you make sure that this does not undo the effect that you wanted
> with your patch?

It will undo it, but you should go ahead and submit this patch. I think we will
then need to add a setter to let embedders customize the background color of the
surfaceView, which I can then call from my Activity to set it back to black.

[Ignacio Solla, 2013-11-28 10:21:12]

On 2013/11/28 10:19:07, igsolla wrote:
> On 2013/11/27 20:52:25, powei wrote:
> > PTAL.  Thanks.
> > 
> > igsolla@ can you make sure that this does not undo the effect that you
wanted
> > with your patch?
> 
> It will undo it, but you should go ahead and submit this patch. I think we
will
> then need to add a setter to let embedders customize the background color of
the
> surfaceView, which I can then call from my Activity to set it back to black.

BTW, you should probably export patch 2 again as the diff is broken and shows
the following message  "error: old chunk mismatch".

[powei, 2013-12-02 20:03:38]

new patchset.  PTAL.  thanks.

@igsolla, I've added a public method for embedders to set to the background
color themselves.

https://codereview.chromium.org/92603002/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
(right):

https://codereview.chromium.org/92603002/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java:63:
setBackgroundColor(Color.WHITE);
On 2013/11/27 21:22:21, sievers wrote:
> I think this can go.

Done.

https://codereview.chromium.org/92603002/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java:288:
mSurfaceView.setBackground(null);
On 2013/11/27 21:22:21, sievers wrote:
> This is only available in JB and up.
> 
> Does mSurfaceView.setBackgroundResource(0) do the trick.
> 
> And then we can remove 287 also.

Done.

[powei, 2013-12-04 20:50:14]

On 2013/12/02 20:03:38, powei wrote:
> new patchset.  PTAL.  thanks.
> 
> @igsolla, I've added a public method for embedders to set to the background
> color themselves.
> 
>
https://codereview.chromium.org/92603002/diff/1/content/public/android/java/s...
> File
>
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
> (right):
> 
>
https://codereview.chromium.org/92603002/diff/1/content/public/android/java/s...
>
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java:63:
> setBackgroundColor(Color.WHITE);
> On 2013/11/27 21:22:21, sievers wrote:
> > I think this can go.
> 
> Done.
> 
>
https://codereview.chromium.org/92603002/diff/1/content/public/android/java/s...
>
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java:288:
> mSurfaceView.setBackground(null);
> On 2013/11/27 21:22:21, sievers wrote:
> > This is only available in JB and up.
> > 
> > Does mSurfaceView.setBackgroundResource(0) do the trick.
> > 
> > And then we can remove 287 also.
> 
> Done.

ping tedchoc@ for review.  Ted, would you mind taking a quick look?  thanks.

[Ted C, 2013-12-04 21:02:24]

lgtm w/ a couple nits

https://codereview.chromium.org/92603002/diff/40001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
(right):

https://codereview.chromium.org/92603002/diff/40001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java:168:
public void setViewBackgroundColor(int color) {
can this be setSurfaceViewBackgroundColor.  Using just View threw me off above
as I thought it was the background color of the ContentViewRenderView.

https://codereview.chromium.org/92603002/diff/40001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java:170:
mSurfaceView.setBackgroundColor(color);
brace are required in java unless the statement and conditional can all fit on a
single line

[powei, 2013-12-04 22:57:59]

comments addressed.  thanks all!.  will try to commit

https://codereview.chromium.org/92603002/diff/40001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
(right):

https://codereview.chromium.org/92603002/diff/40001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java:168:
public void setViewBackgroundColor(int color) {
On 2013/12/04 21:02:24, Ted C wrote:
> can this be setSurfaceViewBackgroundColor.  Using just View threw me off above
> as I thought it was the background color of the ContentViewRenderView.

Done.

https://codereview.chromium.org/92603002/diff/40001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java:170:
mSurfaceView.setBackgroundColor(color);
On 2013/12/04 21:02:24, Ted C wrote:
> brace are required in java unless the statement and conditional can all fit on
a
> single line

Done.

[commit-bot: I haz the power, 2013-12-04 22:59:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/powei@chromium.org/92603002/60001

[commit-bot: I haz the power, 2013-12-05 01:37:15]

Change committed as 238851