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

content/browser/web_contents/web_contents_impl.h

Index: content/browser/web_contents/web_contents_impl.h
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 79c0c86e2b34d8b5df32c2775f16e94cb3222efd..e1d457fcbf483477d0a65c148420a1d72a4fa729 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -728,6 +728,9 @@ class CONTENT_EXPORT WebContentsImpl
     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.
@@ -1166,6 +1169,11 @@ class CONTENT_EXPORT WebContentsImpl
   // 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_;