Link MediaSessionTabHelper with native MediaSession [CL is going to be split]

content/browser/media/session/media_session_observer.cc

+// Copyright 2016 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/media/session/media_session_observer.h"
+
+#include "content/browser/media/session/media_session.h"
+
+namespace content {
+
+MediaSessionObserver::MediaSessionObserver(MediaSession* session)
+    : session_(nullptr) {
+  Observe(session);

[whywhat, 2016/10/20 15:38:33]

don't do that in the ctor - if we ever call any virtual method of MSO from the
same callstack, it won't get handled.
google style guide forbids doing any non-trivial work in ctors for this reason

[whywhat, 2016/10/20 15:38:33]

don't do that in the ctor - if we ever call any virtual method of MSO from the
same callstack, it won't get handled.
google style guide forbids doing any non-trivial work in ctors for this reason
you can make the ctor private and have a create function

[Zhiqiang Zhang (Slow), 2016/10/20 16:22:32]

This pattern was used in WebContentsObserver. Actually Observe() is a private
non-virtual method so it should be fine to call it in the ctor. Since the ctor
has an argument, it must be called by subclass ctors, so all MSO will add itself
to MS in the ctor. Do you have strong opinions on this pattern?

[whywhat, 2016/10/20 18:18:50]

The problem is if Observe results in an event being sent back to the observer
immediately (on the same stack). That will call one of the MSO virtual methods
but the derived class won't be initialized yet.

It's probably hard to fix WebContentsObserver but with MediaRouter & Cast I had
a couple of issues where the event wouldn't fire because of that. Debugging such
kind of things is painful so I'd rather prevent it.

+}
+
+void MediaSessionObserver::Observe(MediaSession* session) {
+  session->AddObserver(this);
+}
+
+}  // namespace content