Mac: Make calling WebContents::WasShown/WasHidden the responsibility of the content layer

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 3103e95e56624064e720877fba1a733c05020155..5456afa8cc54dcbe8db154a17ef96ce27b6c0287 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -338,6 +338,7 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context)
       did_first_visually_non_empty_paint_(false),
       capturer_count_(0),
       should_normally_be_visible_(true),
+      is_or_was_visible_(false),
       is_being_destroyed_(false),
       notify_disconnection_(false),
       dialog_manager_(NULL),
@@ -4798,6 +4799,29 @@ void WebContentsImpl::MediaStoppedPlaying(
   FOR_EACH_OBSERVER(WebContentsObserver, observers_, MediaStoppedPlaying(id));
 }
 
+void WebContentsImpl::UpdateWebContentsVisibility(bool visible) {
+  if (!is_or_was_visible_) {

[Avi (use Gerrit), 2016/02/26 17:50:23]

I'm still struggling here.

This variable starts as false, and gets flipped to true the first time
UpdateWebContentsVisibility is called with true, and then stays as true for the
rest of the WebContents's life? If so, can we name it did_first_set_visible_ or
something like that? There already is a "did_first_visually_non_empty_paint_"
which has a similar flip.

[tapted, 2016/02/29 06:49:00]

Done.

+    // We should not hide the web contents before it was shown the first time,

[Avi (use Gerrit), 2016/02/26 17:50:23]

"we"?

// If this WebContents has not yet been set to be visible for the first time,
// ignore any requests to make it hidden, since resources [...]

[tapted, 2016/02/29 06:49:00]

Done.

+    // since resources would immediately be destroyed and only re-created after
+    // content got loaded. In this state the window content is undefined and can
+    // show garbage.
+    // However - the page load mechanism requires an activation call through a
+    // visibility call to (re)load.
+    if (visible) {
+      is_or_was_visible_ = true;
+      WasShown();
+    }
+    return;
+  }
+  if (visible == should_normally_be_visible_)
+    return;
+
+  if (visible)
+    WasShown();
+  else
+    WasHidden();
+}
+
 void WebContentsImpl::SetJavaScriptDialogManagerForTesting(
     JavaScriptDialogManager* dialog_manager) {
   dialog_manager_ = dialog_manager;