Changed Blimp client to start with white screen before drawing contents

Changed Blimp client to start with white screen before drawing contents

Currently Blimp client starts with a black screen before having any
contents. In this patch, this behavior was fixed by two steps:
1 - BlimpView background color is set to white on initialization.
2 - Later after the first frame is received, the background is reset to
null.
For this to happen, we rely on the callback DidCompleteSwapBuffers of
RemoteChannelImpl and hook it all the way to BlimpView.

BUG=603797
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/a14676fc06d51ce9382217801882dae19e2c792a
Cr-Commit-Position: refs/heads/master@{#391619}

[shaktisahu, 2016-04-28 02:37:15]

Description was changed from

==========
Changed Blimp client to start with white screen before drawing contents

Currently Blimp client starts with a black screen before having any
contents. In this patch, this behavior was fixed by two steps:
1 - BlimpView background color is set to white on initialization.
2 - Later after the first frame is received, the background is reset to
null.
For this to happen, we rely on the callback DidCompleteSwapBuffers of
RemoteChannelImpl and hook it all the way to BlimpView.

BUG=603797
==========

to

==========
Changed Blimp client to start with white screen before drawing contents

Currently Blimp client starts with a black screen before having any
contents. In this patch, this behavior was fixed by two steps:
1 - BlimpView background color is set to white on initialization.
2 - Later after the first frame is received, the background is reset to
null.
For this to happen, we rely on the callback DidCompleteSwapBuffers of
RemoteChannelImpl and hook it all the way to BlimpView.

BUG=603797
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[shaktisahu, 2016-04-28 02:38:06]

shaktisahu@chromium.org changed reviewers:
+ khushalsagar@chromium.org, nyquist@chromium.org

[Khushal, 2016-04-28 03:32:26]

https://codereview.chromium.org/1925863003/diff/1/blimp/client/app/android/bl...
File blimp/client/app/android/blimp_compositor_manager_android.h (right):

https://codereview.chromium.org/1925863003/diff/1/blimp/client/app/android/bl...
blimp/client/app/android/blimp_compositor_manager_android.h:71: BlimpView*
blimp_view_;
I would suggest informing the BlimpView using a client interface instead.

https://codereview.chromium.org/1925863003/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/src/org/chromium/blimp/BlimpView.java
(right):

https://codereview.chromium.org/1925863003/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpView.java:181:
post(new Runnable() {
Is there a reason why this is done as a separate task on this thread?

https://codereview.chromium.org/1925863003/diff/1/cc/trees/remote_channel_imp...
File cc/trees/remote_channel_impl.cc (right):

https://codereview.chromium.org/1925863003/diff/1/cc/trees/remote_channel_imp...
cc/trees/remote_channel_impl.cc:334: void
RemoteChannelImpl::DidCompleteSwapBuffers() {
Could you DCHECK(task_runner_provider_->IsImplThread()) here?

[shaktisahu, 2016-04-28 21:38:16]

https://codereview.chromium.org/1925863003/diff/1/blimp/client/app/android/bl...
File blimp/client/app/android/blimp_compositor_manager_android.h (right):

https://codereview.chromium.org/1925863003/diff/1/blimp/client/app/android/bl...
blimp/client/app/android/blimp_compositor_manager_android.h:71: BlimpView*
blimp_view_;
On 2016/04/28 03:32:26, Khushal wrote:
> I would suggest informing the BlimpView using a client interface instead.

Done.

https://codereview.chromium.org/1925863003/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/src/org/chromium/blimp/BlimpView.java
(right):

https://codereview.chromium.org/1925863003/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpView.java:181:
post(new Runnable() {
On 2016/04/28 03:32:26, Khushal wrote:
> Is there a reason why this is done as a separate task on this thread?

Done. It's better to remove this since we are certain that this is running on
main thread.

https://codereview.chromium.org/1925863003/diff/1/cc/trees/remote_channel_imp...
File cc/trees/remote_channel_impl.cc (right):

https://codereview.chromium.org/1925863003/diff/1/cc/trees/remote_channel_imp...
cc/trees/remote_channel_impl.cc:334: void
RemoteChannelImpl::DidCompleteSwapBuffers() {
On 2016/04/28 03:32:26, Khushal wrote:
> Could you DCHECK(task_runner_provider_->IsImplThread()) here?

Done.

[Khushal, 2016-04-28 21:45:35]

lgtm.

https://codereview.chromium.org/1925863003/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/src/org/chromium/blimp/BlimpView.java
(right):

https://codereview.chromium.org/1925863003/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpView.java:181:
post(new Runnable() {
On 2016/04/28 21:38:15, shaktisahu wrote:
> On 2016/04/28 03:32:26, Khushal wrote:
> > Is there a reason why this is done as a separate task on this thread?
> 
> Done. It's better to remove this since we are certain that this is running on
> main thread.

I actually asked yusuf@ about this and he said its an optimization to not
trigger a redraw from android at the same time as when the compositor is pushing
a frame. But I think since for Blimp the compositor is already running on a
different thread it shouldn't matter.
We can add it later if necessary.

[shaktisahu, 2016-04-28 22:18:37]

shaktisahu@chromium.org changed reviewers:
+ vmpstr@chromium.org

[shaktisahu, 2016-04-28 22:18:38]

vmpstr@chromium.org: Please review changes in

[vmpstr, 2016-04-29 23:19:40]

cc lgtm

[nyquist, 2016-05-03 22:40:37]

lgtm

https://codereview.chromium.org/1925863003/diff/20001/blimp/client/app/androi...
File blimp/client/app/android/java/src/org/chromium/blimp/BlimpView.java
(right):

https://codereview.chromium.org/1925863003/diff/20001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpView.java:180: if
(getBackground() != null) {
Nit: Could you flip this?
if (getBackground() == null) return;

[shaktisahu, 2016-05-03 23:24:52]

https://codereview.chromium.org/1925863003/diff/20001/blimp/client/app/androi...
File blimp/client/app/android/java/src/org/chromium/blimp/BlimpView.java
(right):

https://codereview.chromium.org/1925863003/diff/20001/blimp/client/app/androi...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpView.java:180: if
(getBackground() != null) {
On 2016/05/03 22:40:37, nyquist wrote:
> Nit: Could you flip this?
> if (getBackground() == null) return;

Done.

[shaktisahu, 2016-05-03 23:25:17]

The CQ bit was checked by shaktisahu@chromium.org

[shaktisahu, 2016-05-03 23:25:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
nyquist@chromium.org, vmpstr@chromium.org, khushalsagar@chromium.org
Link to the patchset: https://codereview.chromium.org/1925863003/#ps40001
(title: "A few nits")

[commit-bot: I haz the power, 2016-05-03 23:25:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1925863003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1925863003/40001

[commit-bot: I haz the power, 2016-05-04 01:21:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-04 01:21:59]

Failed to apply the patch.

[commit-bot: I haz the power, 2016-05-04 01:22:10]

The CQ bit was unchecked by commit-bot@chromium.org

[shaktisahu, 2016-05-04 19:09:38]

The CQ bit was checked by shaktisahu@chromium.org

[shaktisahu, 2016-05-04 19:09:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
nyquist@chromium.org, vmpstr@chromium.org, khushalsagar@chromium.org
Link to the patchset: https://codereview.chromium.org/1925863003/#ps60001
(title: "merge origin/master")

[commit-bot: I haz the power, 2016-05-04 19:10:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1925863003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1925863003/60001

[commit-bot: I haz the power, 2016-05-04 20:11:09]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-04 20:13:54]

Description was changed from

==========
Changed Blimp client to start with white screen before drawing contents

Currently Blimp client starts with a black screen before having any
contents. In this patch, this behavior was fixed by two steps:
1 - BlimpView background color is set to white on initialization.
2 - Later after the first frame is received, the background is reset to
null.
For this to happen, we rely on the callback DidCompleteSwapBuffers of
RemoteChannelImpl and hook it all the way to BlimpView.

BUG=603797
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Changed Blimp client to start with white screen before drawing contents

Currently Blimp client starts with a black screen before having any
contents. In this patch, this behavior was fixed by two steps:
1 - BlimpView background color is set to white on initialization.
2 - Later after the first frame is received, the background is reset to
null.
For this to happen, we rely on the callback DidCompleteSwapBuffers of
RemoteChannelImpl and hook it all the way to BlimpView.

BUG=603797
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/a14676fc06d51ce9382217801882dae19e2c792a
Cr-Commit-Position: refs/heads/master@{#391619}
==========

[commit-bot: I haz the power, 2016-05-04 20:13:56]

Patchset 4 (id:??) landed as
https://crrev.com/a14676fc06d51ce9382217801882dae19e2c792a
Cr-Commit-Position: refs/heads/master@{#391619}

[tommycli, 2016-05-10 17:02:54]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1965843002/ by tommycli@chromium.org.

The reason for reverting is: Reverting because this patch broke the Dr Memory
bot:

https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%2....