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

content/browser/web_contents/web_contents_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_WEB_CONTENTS_WEB_CONTENTS_IMPL_H_
 #define CONTENT_BROWSER_WEB_CONTENTS_WEB_CONTENTS_IMPL_H_
 
 #include <stdint.h>
 
 #include <map>
@@ skipping 710 matching lines @@
 
   // Called by MediaWebContentsObserver when playback starts or stops.  See the
   // WebContentsObserver function stubs for more details.
   void MediaStartedPlaying(const WebContentsObserver::MediaPlayerId& id);
   void MediaStoppedPlaying(const WebContentsObserver::MediaPlayerId& id);
 
   MediaWebContentsObserver* media_web_contents_observer() {
     return media_web_contents_observer_.get();
   }
 
+  // Update the web contents visibility.
+  void UpdateWebContentsVisibility(bool visible);
+
  private:
   friend class WebContentsObserver;
   friend class WebContents;  // To implement factory methods.
 
   FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, NoJSMessageOnInterstitials);
   FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, UpdateTitle);
   FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, FindOpenerRVHWhenPending);
   FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest,
                            CrossSiteCantPreemptAfterUnload);
   FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, PendingContents);
@@ skipping 418 matching lines @@
   // Data for misc internal state ----------------------------------------------
 
   // When > 0, the WebContents is currently being captured (e.g., for
   // screenshots or mirroring); and the underlying RenderWidgetHost should not
   // be told it is hidden.
   int capturer_count_;
 
   // Tracks whether RWHV should be visible once capturer_count_ becomes zero.
   bool should_normally_be_visible_;
 
+  // Background tabs are loaded with |should_normally_be_visible_| set, so that
+  // they actually load something, but then need to receive an initial
+  // WasShown() message when first switching to that tab.

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

This is a rationale of _why_ we need this, but it's not clear what this boolean
holds. Please make the comment explain _what_ is held in this variable, and save
the _why_ for the function that manipulates it.

Perhaps:

// This tracks whether this WebContents was ever set to be visible.
bool did_first_set_visible_;

or something like that. I think that's its meaning, though I'm still not
entirely sure.

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

Done. Clarified it a bit. When it was just used for aura, the comment went "On
Windows we can run into problems if resources get released within the
initialization phase while the content (and its dimensions) are not known."
Which is a bit vague.

Also, on Mac, it was clear things were relying on the initial WasShown which
could be suppressed without this.

I did a bit more spelunking - this came about in r280187 - included a bit more
so it's clear what it's for.

+  bool is_or_was_visible_;
+
   // See getter above.
   bool is_being_destroyed_;
 
   // Indicates whether we should notify about disconnection of this
   // WebContentsImpl. This is used to ensure disconnection notifications only
   // happen if a connection notification has happened and that they happen only
   // once.
   bool notify_disconnection_;
 
   // Pointer to the JavaScript dialog manager, lazily assigned. Used because the
@@ skipping 162 matching lines @@
   // Adds/removes a callback called on creation of each new WebContents.
   static void AddCreatedCallbackForTesting(const CreatedCallback& callback);
   static void RemoveCreatedCallbackForTesting(const CreatedCallback& callback);
 
   DISALLOW_COPY_AND_ASSIGN(FriendZone);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_WEB_CONTENTS_WEB_CONTENTS_IMPL_H_