Clean up AwContents calls to change visibility

Clean up AwContents calls to change visibility

Visibility is stored in AwContents and native BrowserViewRenderer. It
seems to be windowVisible && viewVisible && !mIsPaused. So clean all
this up, make sure we don't make unnecessary calls into ContentViewCore.

Precursor work to pass visibility from BrowserViewRenderer directly to
cc.

BUG=250594
NOTRY=true

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

[boliu, 2013-06-20 18:40:35]

focus may need clean up too...

[benm (inactive), 2013-06-20 18:45:45]

https://codereview.chromium.org/17291010/diff/1/android_webview/java/src/org/...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/17291010/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1182:
mContentViewCore.onShow();
If |forced| is true, then we might end up calling onShow or onHide when the CVC
is already in that state. Is that OK?

[boliu, 2013-06-20 18:49:55]

https://codereview.chromium.org/17291010/diff/1/android_webview/java/src/org/...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/17291010/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1182:
mContentViewCore.onShow();
On 2013/06/20 18:45:45, benm wrote:
> If |forced| is true, then we might end up calling onShow or onHide when the
CVC
> is already in that state. Is that OK?

Forced is only used in pop up window case, that's when we get a new WebContents,
so I think it's safe here.

It's definitely not worse than before when we call onShow/Hide multiple times
anyway.

[benm (inactive), 2013-06-20 19:22:28]

lgtm % typo

https://codereview.chromium.org/17291010/diff/1/android_webview/java/src/org/...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/17291010/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:127:
private boolean mIsVisible;  // Equivalment to windowVisible && viewVisible &&
!mIsPaused.
sp equivalent

[boliu, 2013-06-20 20:21:01]

https://codereview.chromium.org/17291010/diff/1/android_webview/java/src/org/...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/17291010/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:127:
private boolean mIsVisible;  // Equivalment to windowVisible && viewVisible &&
!mIsPaused.
On 2013/06/20 19:22:28, benm wrote:
> sp equivalent

Done.

[joth, 2013-06-20 22:15:12]

lgtm

[commit-bot: I haz the power, 2013-06-21 01:44:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/boliu@chromium.org/17291010/7001

[commit-bot: I haz the power, 2013-06-21 01:44:14]

Failed to apply patch for android_webview/browser/in_process_view_renderer.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file android_webview/browser/in_process_view_renderer.cc
  Hunk #1 FAILED at 305.
  Hunk #2 succeeded at 364 (offset 2 lines).
  Hunk #3 succeeded at 553 (offset 2 lines).
  Hunk #4 succeeded at 600 (offset 2 lines).
  1 out of 4 hunks FAILED -- saving rejects to file
android_webview/browser/in_process_view_renderer.cc.rej

Patch:       android_webview/browser/in_process_view_renderer.cc
Index: android_webview/browser/in_process_view_renderer.cc
diff --git a/android_webview/browser/in_process_view_renderer.cc
b/android_webview/browser/in_process_view_renderer.cc
index
28810e2b7aa3c5d303bed9a3ed1b2fcea98ed103..9fa906eb8e216de32026590455cafaf4e24749a5
100644
--- a/android_webview/browser/in_process_view_renderer.cc
+++ b/android_webview/browser/in_process_view_renderer.cc
@@ -305,7 +305,7 @@ InProcessViewRenderer::InProcessViewRenderer(
       java_helper_(java_helper),
       web_contents_(web_contents),
       compositor_(NULL),
-      view_visible_(false),
+      visible_(false),
       continuous_invalidate_(false),
       block_invalidates_(false),
       width_(0),
@@ -362,7 +362,7 @@ void InProcessViewRenderer::DrawGL(AwDrawGLInfo* draw_info)
{
     return;
 
   TRACE_EVENT0("android_webview", "InProcessViewRenderer::DrawGL");
-  DCHECK(view_visible_);
+  DCHECK(visible_);
 
   // We need to watch if the current Android context has changed and enforce
   // a clean-up in the compositor.
@@ -551,16 +551,13 @@ InProcessViewRenderer::CapturePicture() {
 void InProcessViewRenderer::EnableOnNewPicture(bool enabled) {
 }
 
-void InProcessViewRenderer::OnVisibilityChanged(bool view_visible,
-                                                bool window_visible) {
-  TRACE_EVENT_INSTANT2("android_webview",
+void InProcessViewRenderer::OnVisibilityChanged(bool visible) {
+  TRACE_EVENT_INSTANT1("android_webview",
                        "InProcessViewRenderer::OnVisibilityChanged",
                        TRACE_EVENT_SCOPE_THREAD,
-                       "view_visible",
-                       view_visible,
-                       "window_visible",
-                       window_visible);
-  view_visible_ = window_visible && view_visible;
+                       "visible",
+                       visible);
+  visible_ = visible;
 }
 
 void InProcessViewRenderer::OnSizeChanged(int width, int height) {
@@ -601,7 +598,7 @@ bool InProcessViewRenderer::IsAttachedToWindow() {
 }
 
 bool InProcessViewRenderer::IsViewVisible() {
-  return view_visible_;
+  return visible_;
 }
 
 gfx::Rect InProcessViewRenderer::GetScreenRect() {

[commit-bot: I haz the power, 2013-06-21 05:41:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/boliu@chromium.org/17291010/14001

[commit-bot: I haz the power, 2013-06-21 05:43:05]

Change committed as 207720