Inform MediaObserver when video natural size changes.

media/blink/webmediaplayer_impl.cc

 // Copyright 2013 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 "media/blink/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <string>
@@ skipping 1300 matching lines @@
 
   if (overlay_enabled_ && surface_manager_)
     surface_manager_->NaturalSizeChanged(rotated_size);
 
   if (pipeline_metadata_.natural_size.IsEmpty()) {
     // WatchTimeReporter doesn't report metrics for empty videos. Re-create
     // |watch_time_reporter_| if we didn't originally know the video size.
     CreateWatchTimeReporter();
   }
 
   pipeline_metadata_.natural_size = rotated_size;

[sandersd (OOO until July 31), 2017/01/06 19:29:18]

Looks like there is a pre-existing bug here, CreateWatchTimeReporter() shouldn't
be called until after |pipeline_metadata_.natural_size| is updated.

Please fix.

[xjz, 2017/01/06 21:59:15]

Done.

   client_->sizeChanged();
+
+  if (observer_) {
+    PipelineMetadata metadata = pipeline_metadata_;
+    metadata.natural_size = size;

[sandersd (OOO until July 31), 2017/01/06 19:29:18]

Almost certainly you want the rotated_size. Since that's what's in
|pipeline_metadata_|, just pass that?

[xjz, 2017/01/06 21:59:15]

The size will be rotated in RemotingRendererController if needed
(https://cs.chromium.org/chromium/src/media/remoting/remoting_renderer_control...).
Therefore pass the original size here. Actually it is confusing to have some
PipelineMetadata store orginal natural size, and some store the rotated size.
Since now all application requires roted size, how about let it always store the
rotated size? That is, we only need to rotate it once in PipelineImpl when this
metadata is created. If this sgty, I can do the refactoring in a separate CL.

[sandersd (OOO until July 31), 2017/01/06 22:18:07]

OnMetadata() also applies the rotation, so if RemotingRendererController is also
applying a rotation it's probably wrong.

Moving this up the stack in a followup SGTM.

[xjz, 2017/01/06 22:23:57]

OnMetadata() also passed the original size (not the rotated one) to
MediaObserver
(https://cs.chromium.org/chromium/src/media/blink/webmediaplayer_impl.cc?rcl=1...).
I will do a follow up refactoring CL.

+    observer_->OnMetadataChanged(metadata);
+  }
 }
 
 void WebMediaPlayerImpl::OnVideoOpacityChange(bool opaque) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   DCHECK_NE(ready_state_, WebMediaPlayer::ReadyStateHaveNothing);
 
   opaque_ = opaque;
   // Modify content opaqueness of cc::Layer directly so that
   // SetContentsOpaqueIsFixed is ignored.
   if (video_weblayer_)
@@ skipping 701 matching lines @@
   return pipeline_metadata_.video_rotation == VIDEO_ROTATION_0;
 }
 
 void WebMediaPlayerImpl::ActivateViewportIntersectionMonitoring(bool activate) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   client_->activateViewportIntersectionMonitoring(activate);
 }
 
 }  // namespace media