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

content/browser/web_contents/web_contents_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/web_contents/web_contents_impl.h"
 
 #include <stddef.h>
 
 #include <cmath>
 #include <utility>
@@ skipping 320 matching lines @@
       upload_size_(0),
       upload_position_(0),
       is_resume_pending_(false),
       displayed_insecure_content_(false),
       has_accessed_initial_document_(false),
       theme_color_(SK_ColorTRANSPARENT),
       last_sent_theme_color_(SK_ColorTRANSPARENT),
       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),
       is_showing_before_unload_dialog_(false),
       last_active_time_(base::TimeTicks::Now()),
       closed_by_user_gesture_(false),
       minimum_zoom_percent_(static_cast<int>(kMinimumZoomFactor * 100)),
       maximum_zoom_percent_(static_cast<int>(kMaximumZoomFactor * 100)),
       zoom_scroll_remainder_(0),
       render_view_message_source_(NULL),
@@ skipping 4440 matching lines @@
 void WebContentsImpl::MediaStartedPlaying(
     const WebContentsObserver::MediaPlayerId& id) {
   FOR_EACH_OBSERVER(WebContentsObserver, observers_, MediaStartedPlaying(id));
 }
 
 void WebContentsImpl::MediaStoppedPlaying(
     const WebContentsObserver::MediaPlayerId& id) {
   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;
 }
 
 }  // namespace content