rendering_helper - Change the layout of the rendering areas.

rendering_helper - Change the layout of the rendering areas.

1. Layout the rendering areas in grid.
2. Shrink the video to match the screen size.

BUG=None
TEST=Run the test on peach-pit

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277364

[Owen Lin, 2014-05-27 05:56:53]

PTAL. Thanks.

[Ami GONE FROM CHROMIUM, 2014-05-27 16:05:24]

Nice simplification!

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
File content/common/gpu/media/rendering_helper.cc (right):

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
content/common/gpu/media/rendering_helper.cc:581: // calculate the offsets.
This comment isn't adding value because the non-cognoscenti will not know what
"scale" or "calculate" mean in this context.  It would be more useful to
describe the desired outcome which this function brings about.

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
content/common/gpu/media/rendering_helper.cc:582: static void
ScaleAndCalculateOffsets(std::vector<int>& lengths,
const ref or non-const ptr, never non-const ref.
here and elsewhere

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
content/common/gpu/media/rendering_helper.cc:589: offsets[i] = offsets[i - 1] +
lengths[i - 1];
else
offsets[0] = 0;

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
content/common/gpu/media/rendering_helper.cc:604: }
are l.594-604 equiv to
size_t rows = sqrt(clients_.size());
size_t cols = (clients_.size() + rows - 1) / rows;
?

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
content/common/gpu/media/rendering_helper.cc:624: const gfx::Size &window_size =
clients_[i]->GetWindowSize();
nit: & sticks left not right

[Owen Lin, 2014-05-28 06:55:17]

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
File content/common/gpu/media/rendering_helper.cc (right):

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
content/common/gpu/media/rendering_helper.cc:581: // calculate the offsets.
On 2014/05/27 16:05:25, Ami Fischman wrote:
> This comment isn't adding value because the non-cognoscenti will not know what
> "scale" or "calculate" mean in this context.  It would be more useful to
> describe the desired outcome which this function brings about.

The comment is modified. Thanks.

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
content/common/gpu/media/rendering_helper.cc:582: static void
ScaleAndCalculateOffsets(std::vector<int>& lengths,
On 2014/05/27 16:05:25, Ami Fischman wrote:
> const ref or non-const ptr, never non-const ref.
> here and elsewhere

Done.

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
content/common/gpu/media/rendering_helper.cc:589: offsets[i] = offsets[i - 1] +
lengths[i - 1];
On 2014/05/27 16:05:25, Ami Fischman wrote:
> else
> offsets[0] = 0;

Done.

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
content/common/gpu/media/rendering_helper.cc:604: }
On 2014/05/27 16:05:25, Ami Fischman wrote:
> are l.594-604 equiv to
> size_t rows = sqrt(clients_.size());
> size_t cols = (clients_.size() + rows - 1) / rows;
> ?

Done. Thanks.

https://codereview.chromium.org/298123003/diff/20001/content/common/gpu/media...
content/common/gpu/media/rendering_helper.cc:624: const gfx::Size &window_size =
clients_[i]->GetWindowSize();
On 2014/05/27 16:05:25, Ami Fischman wrote:
> nit: & sticks left not right

Done.

[Ami GONE FROM CHROMIUM, 2014-05-28 16:03:32]

lgtm

[Owen Lin, 2014-06-11 06:45:24]

Hi, 

Ami has leaved the team and no longer the owner. Can you lgtm this change?
Thanks.

[scherkus (not reviewing), 2014-06-11 21:32:29]

lgtm

[Owen Lin, 2014-06-12 08:56:26]

The CQ bit was checked by owenlin@chromium.org

[commit-bot: I haz the power, 2014-06-12 09:00:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/owenlin@chromium.org/298123003/60001

[Owen Lin, 2014-06-12 09:39:34]

The CQ bit was unchecked by owenlin@chromium.org

[Owen Lin, 2014-06-12 09:45:59]

The CQ bit was checked by owenlin@chromium.org

[commit-bot: I haz the power, 2014-06-12 09:49:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/owenlin@chromium.org/298123003/80001

[commit-bot: I haz the power, 2014-06-12 13:49:46]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-06-12 14:31:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-12 14:31:03]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[Owen Lin, 2014-06-16 02:23:52]

The CQ bit was checked by owenlin@chromium.org

[commit-bot: I haz the power, 2014-06-16 02:24:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/owenlin@chromium.org/298123003/100001

[commit-bot: I haz the power, 2014-06-16 07:12:51]

Change committed as 277364