Chromium changes for static linking ffmpeg

Chromium changes to statically link ffmpeg.

Motivations:
1. Eliminates a large number of windows crashes hit while loading the DLL (see bugs)
2. Reducing the size of binary (about 400KB on linux, about 265KB on 32bit windows) due to dead-code elimination
3. Simplifying media library initialization (Remove bool-ness, it just works)

Licensing:
We've audited all the licenses for the files we actually use and have the green light from legal as long as we include the license text in the credits.

BUG=435455, 429131, 441908
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/bfef6d6ba82b172ac0e48c80d9ccf1a1ca0ea5c5
Cr-Commit-Position: refs/heads/master@{#332891}

Committed: https://crrev.com/fd11b3c3aa6730ae1f024811b61913cd63f9d39a
Cr-Commit-Position: refs/heads/master@{#333423}

[DaleCurtis, 2015-05-15 00:23:04]

dalecurtis@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2015-05-15 00:23:06]

https://codereview.chromium.org/1141703002/diff/160001/media/base/media.cc
File media/base/media.cc (right):

https://codereview.chromium.org/1141703002/diff/160001/media/base/media.cc#ne...
media/base/media.cc:61: bool InitializeMediaLibrary(const base::FilePath&
module_dir) {
Delete?

https://codereview.chromium.org/1141703002/diff/160001/media/base/media.h
File media/base/media.h (right):

https://codereview.chromium.org/1141703002/diff/160001/media/base/media.h#new...
media/base/media.h:21: // TODO(chcunningham): Remove this method in favor of the
zero-argument version.
Looks like you can delete all these methods within this patch set except the
void() one and the CPU specific one, no?

https://codereview.chromium.org/1141703002/diff/160001/media/base/media_stub.cc
File media/base/media_stub.cc (left):

https://codereview.chromium.org/1141703002/diff/160001/media/base/media_stub....
media/base/media_stub.cc:1: // Copyright 2012 The Chromium Authors. All rights
reserved.
Don't you need this on android?

https://codereview.chromium.org/1141703002/diff/160001/media/base/media_win.cc
File media/base/media_win.cc (left):

https://codereview.chromium.org/1141703002/diff/160001/media/base/media_win.c...
media/base/media_win.cc:41:
UMA_HISTOGRAM_SPARSE_SLOWLY("Media.Initialize.Windows", GetLastError());
Need to mark this histogram as deprecated now.

https://codereview.chromium.org/1141703002/diff/160001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1141703002/diff/160001/media/media.gyp#newcod...
media/media.gyp:1264: ['OS=="win"', {
Just combine this with l.1255?

[chcunningham, 2015-05-15 23:42:48]

chcunningham@chromium.org changed reviewers:
+ maruel@chromium.org, mmoss@chromium.org

[chcunningham, 2015-05-15 23:42:49]

mauel@/mmoss@ can you lgtm for the .isolate files?

[M-A Ruel, 2015-05-15 23:51:29]

The CL description is very dry.

https://codereview.chromium.org/1141703002/diff/220001/media/base/media.cc
File media/base/media.cc (right):

https://codereview.chromium.org/1141703002/diff/220001/media/base/media.cc#ne...
media/base/media.cc:18: extern void InitializeMediaLibraryInternal();
The function is not external. Also per coding style we do not do this kind of
things and always use headers instead.

https://codereview.chromium.org/1141703002/diff/220001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1141703002/diff/220001/media/media.gyp#newcod...
media/media.gyp:316: 'base/media_init.cc',
sort

https://codereview.chromium.org/1141703002/diff/220001/media/media.gyp#newcod...
media/media.gyp:617: 'base/media_init.cc',
sort

https://codereview.chromium.org/1141703002/diff/220001/media/media.gyp#newcod...
media/media.gyp:1264: ['OS=="win"', {
['OS=="win" and win_use_allocator_shim==1', {

I don't see any reason to use embedded conditions.

https://codereview.chromium.org/1141703002/diff/220001/third_party/x86inc/x86...
File third_party/x86inc/x86inc.asm (right):

https://codereview.chromium.org/1141703002/diff/220001/third_party/x86inc/x86...
third_party/x86inc/x86inc.asm:516: global %1:function hidden
This change doesn't seem related with the rest.

[DaleCurtis, 2015-05-15 23:57:45]

https://codereview.chromium.org/1141703002/diff/220001/media/base/media.cc
File media/base/media.cc (right):

https://codereview.chromium.org/1141703002/diff/220001/media/base/media.cc#ne...
media/base/media.cc:18: extern void InitializeMediaLibraryInternal();
On 2015/05/15 23:51:29, M-A Ruel wrote:
> The function is not external. Also per coding style we do not do this kind of
> things and always use headers instead.

Yeah this isn't really necessary anymore since we have just android and the
rest.

I'd just delete media_init.cc and media_init_stub.cc and just use a #if
defined(OS_ANDROID) {} else { av_xxx() }.

https://codereview.chromium.org/1141703002/diff/220001/third_party/x86inc/x86...
File third_party/x86inc/x86inc.asm (right):

https://codereview.chromium.org/1141703002/diff/220001/third_party/x86inc/x86...
third_party/x86inc/x86inc.asm:516: global %1:function hidden
On 2015/05/15 23:51:29, M-A Ruel wrote:
> This change doesn't seem related with the rest.

Yeah this should be dropped, only the x86inc.asm inside of ffmpeg needs
tweaking.

[chcunningham, 2015-05-16 00:02:49]

chcunningham@chromium.org changed reviewers:
+ gab@chromium.org, grt@chromium.org, phajdan@google.com, sky@chromium.org,
thomasvl@chromium.org

[chcunningham, 2015-05-16 00:02:50]

Adding a few more owners :)

gab & grt for the installer files
phajdan & sky for the autotest files
mmoss for the build/*/FILES.cfg
thomasvl for the mac/TESTS

[chcunningham, 2015-05-16 04:36:26]

Thanks both. Added some detail to the change description.

https://codereview.chromium.org/1141703002/diff/160001/media/base/media.cc
File media/base/media.cc (right):

https://codereview.chromium.org/1141703002/diff/160001/media/base/media.cc#ne...
media/base/media.cc:61: bool InitializeMediaLibrary(const base::FilePath&
module_dir) {
On 2015/05/15 00:23:06, DaleCurtis wrote:
> Delete?

This is still called by a number of people outside media. I just shimmed it for
now to limit the scope of this change. Will definitely delete it later on.

https://codereview.chromium.org/1141703002/diff/160001/media/base/media.h
File media/base/media.h (right):

https://codereview.chromium.org/1141703002/diff/160001/media/base/media.h#new...
media/base/media.h:21: // TODO(chcunningham): Remove this method in favor of the
zero-argument version.
On 2015/05/15 00:23:06, DaleCurtis wrote:
> Looks like you can delete all these methods within this patch set except the
> void() one and the CPU specific one, no?

I haven't changed places like here yet.
https://code.google.com/p/chromium/codesearch#chromium/src/content/app/conten...

For a later cl if you agree.

https://codereview.chromium.org/1141703002/diff/160001/media/base/media_stub.cc
File media/base/media_stub.cc (left):

https://codereview.chromium.org/1141703002/diff/160001/media/base/media_stub....
media/base/media_stub.cc:1: // Copyright 2012 The Chromium Authors. All rights
reserved.
On 2015/05/15 00:23:05, DaleCurtis wrote:
> Don't you need this on android?

This is now done in media_init_stub.cc. Its essentially the same, but the
structure is a little different since we dont need the path, nor the bool
return. I changed the names a little. We used to have options paths (_posix,
_media, and _stubs), now just two (_init, and init_stubs)

https://codereview.chromium.org/1141703002/diff/160001/media/base/media_win.cc
File media/base/media_win.cc (left):

https://codereview.chromium.org/1141703002/diff/160001/media/base/media_win.c...
media/base/media_win.cc:41:
UMA_HISTOGRAM_SPARSE_SLOWLY("Media.Initialize.Windows", GetLastError());
On 2015/05/15 00:23:05, DaleCurtis wrote:
> Need to mark this histogram as deprecated now.

Done.

https://codereview.chromium.org/1141703002/diff/160001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1141703002/diff/160001/media/media.gyp#newcod...
media/media.gyp:1264: ['OS=="win"', {
On 2015/05/15 00:23:06, DaleCurtis wrote:
> Just combine this with l.1255?

Done.

https://codereview.chromium.org/1141703002/diff/220001/media/base/media.cc
File media/base/media.cc (right):

https://codereview.chromium.org/1141703002/diff/220001/media/base/media.cc#ne...
media/base/media.cc:18: extern void InitializeMediaLibraryInternal();
On 2015/05/15 23:57:45, DaleCurtis wrote:
> On 2015/05/15 23:51:29, M-A Ruel wrote:
> > The function is not external. Also per coding style we do not do this kind
of
> > things and always use headers instead.
> 
> Yeah this isn't really necessary anymore since we have just android and the
> rest.
> 
> I'd just delete media_init.cc and media_init_stub.cc and just use a #if
> defined(OS_ANDROID) {} else { av_xxx() }.

Done. I used the existing define MEDIA_DISABLE_FFMPEG - set for both ios and
android.

https://codereview.chromium.org/1141703002/diff/220001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1141703002/diff/220001/media/media.gyp#newcod...
media/media.gyp:316: 'base/media_init.cc',
On 2015/05/15 23:51:29, M-A Ruel wrote:
> sort

deleted.

https://codereview.chromium.org/1141703002/diff/220001/media/media.gyp#newcod...
media/media.gyp:617: 'base/media_init.cc',
On 2015/05/15 23:51:29, M-A Ruel wrote:
> sort

deleted.

https://codereview.chromium.org/1141703002/diff/220001/media/media.gyp#newcod...
media/media.gyp:1264: ['OS=="win"', {
On 2015/05/15 23:51:29, M-A Ruel wrote:
> ['OS=="win" and win_use_allocator_shim==1', {
> 
> I don't see any reason to use embedded conditions.

ended up combining this with the above condition

https://codereview.chromium.org/1141703002/diff/220001/third_party/x86inc/x86...
File third_party/x86inc/x86inc.asm (right):

https://codereview.chromium.org/1141703002/diff/220001/third_party/x86inc/x86...
third_party/x86inc/x86inc.asm:516: global %1:function hidden
On 2015/05/15 23:57:45, DaleCurtis wrote:
> On 2015/05/15 23:51:29, M-A Ruel wrote:
> > This change doesn't seem related with the rest.
> 
> Yeah this should be dropped, only the x86inc.asm inside of ffmpeg needs
> tweaking.

Dropped.

[DaleCurtis, 2015-05-18 23:33:14]

https://codereview.chromium.org/1141703002/diff/220001/media/base/media.cc
File media/base/media.cc (right):

https://codereview.chromium.org/1141703002/diff/220001/media/base/media.cc#ne...
media/base/media.cc:18: extern void InitializeMediaLibraryInternal();
On 2015/05/16 04:36:25, chcunningham wrote:
> On 2015/05/15 23:57:45, DaleCurtis wrote:
> > On 2015/05/15 23:51:29, M-A Ruel wrote:
> > > The function is not external. Also per coding style we do not do this kind
> of
> > > things and always use headers instead.
> > 
> > Yeah this isn't really necessary anymore since we have just android and the
> > rest.
> > 
> > I'd just delete media_init.cc and media_init_stub.cc and just use a #if
> > defined(OS_ANDROID) {} else { av_xxx() }.
> 
> Done. I used the existing define MEDIA_DISABLE_FFMPEG - set for both ios and
> android.

Doesn't seem deleted yet?

https://codereview.chromium.org/1141703002/diff/350001/content/app/content_ma...
File content/app/content_main_runner.cc (left):

https://codereview.chromium.org/1141703002/diff/350001/content/app/content_ma...
content/app/content_main_runner.cc:287: if (PathService::Get(DIR_MEDIA_LIBS,
&media_path))
You can remove the DIR_MEDIA_LIBS PathService entries now too from
content/common/content_paths.{h,cc}

[Paweł Hajdan Jr., 2015-05-19 09:21:33]

phajdan.jr@chromium.org changed reviewers:
+ phajdan.jr@chromium.org
- phajdan@google.com

[Paweł Hajdan Jr., 2015-05-19 09:21:34]

LGTM

[grt (UTC plus 2), 2015-05-19 16:33:51]

On 2015/05/16 00:02:50, chcunningham wrote:
> Adding a few more owners :)
> 
> gab & grt for the installer files

Please ping me to take a look when the trybots are green. Thanks.

[gab, 2015-05-19 19:41:50]

On 2015/05/19 16:33:51, grt wrote:
> On 2015/05/16 00:02:50, chcunningham wrote:
> > Adding a few more owners :)
> > 
> > gab & grt for the installer files
> 
> Please ping me to take a look when the trybots are green. Thanks.

Also, one of us is sufficient for installer/, flooding owners will only make it
blurry who should review what and delay throughput (on this, removing myself).

[gab, 2015-05-19 19:42:07]

gab@chromium.org changed reviewers:
- gab@chromium.org

[chcunningham, 2015-05-21 22:10:27]

Addressing some feedback, fixing some issues (though some remain).

With this last patch, I expect test failures for GYP component builds (segfault
from tcmalloc). The same tests pass for GN component builds and I've not yet
tracked down the difference. I do note that the libffmpegsumo.so is ~2x the size
in the GYP debug build - pretty strong hint... still tracking it down.

Also, the mac rel build will likely still fail. When I do this build locally it
passes (and passed for Dale too). Not sure yet what I'm missing.

https://codereview.chromium.org/1141703002/diff/220001/media/base/media.cc
File media/base/media.cc (right):

https://codereview.chromium.org/1141703002/diff/220001/media/base/media.cc#ne...
media/base/media.cc:18: extern void InitializeMediaLibraryInternal();
On 2015/05/18 23:33:14, DaleCurtis_OOO_Until_May_26 wrote:
> On 2015/05/16 04:36:25, chcunningham wrote:
> > On 2015/05/15 23:57:45, DaleCurtis wrote:
> > > On 2015/05/15 23:51:29, M-A Ruel wrote:
> > > > The function is not external. Also per coding style we do not do this
kind
> > of
> > > > things and always use headers instead.
> > > 
> > > Yeah this isn't really necessary anymore since we have just android and
the
> > > rest.
> > > 
> > > I'd just delete media_init.cc and media_init_stub.cc and just use a #if
> > > defined(OS_ANDROID) {} else { av_xxx() }.
> > 
> > Done. I used the existing define MEDIA_DISABLE_FFMPEG - set for both ios and
> > android.
> 
> Doesn't seem deleted yet?

Woops, fixed.

https://codereview.chromium.org/1141703002/diff/350001/content/app/content_ma...
File content/app/content_main_runner.cc (left):

https://codereview.chromium.org/1141703002/diff/350001/content/app/content_ma...
content/app/content_main_runner.cc:287: if (PathService::Get(DIR_MEDIA_LIBS,
&media_path))
On 2015/05/18 23:33:14, DaleCurtis_OOO_Until_May_26 wrote:
> You can remove the DIR_MEDIA_LIBS PathService entries now too from
> content/common/content_paths.{h,cc}

This seems to still be used here:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/utility/med...

[chcunningham, 2015-06-02 17:41:03]

Hey All, Thanks for your patience. The bots are finally green. Let the reviews
commence! 

Caveat: mac_chromium_dbg is purple. Its purple for a lot of people recently, so
this is likely unrelated infra issue.
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_dbg_ng/

[DaleCurtis, 2015-06-02 18:21:07]

lgtm % some nits and cleanup. Thanks Chris!

https://codereview.chromium.org/1141703002/diff/510001/chrome/utility/extensi...
File chrome/utility/extensions/extensions_handler.cc (right):

https://codereview.chromium.org/1141703002/diff/510001/chrome/utility/extensi...
chrome/utility/extensions/extensions_handler.cc:68: // Initialize media
libraries for media file validation.
Doesn't really need to be pre-sandbox startup anymore; we should find a better
location for this, but it's fine for now.

https://codereview.chromium.org/1141703002/diff/510001/media/base/media.cc
File media/base/media.cc (right):

https://codereview.chromium.org/1141703002/diff/510001/media/base/media.cc#ne...
media/base/media.cc:23: void Initialize() {
No need for an Initilize() method and a lock anymore, this can be condensed into
the constructor and initialized_ deleted.

https://codereview.chromium.org/1141703002/diff/510001/media/base/media.cc#ne...
media/base/media.cc:27: // Disable logging as it interferes with layout tests.
Indent is wrong for these, it should be as if the #if wasn't there.

https://codereview.chromium.org/1141703002/diff/510001/media/ffmpeg/ffmpeg_co...
File media/ffmpeg/ffmpeg_common_unittest.cc (right):

https://codereview.chromium.org/1141703002/diff/510001/media/ffmpeg/ffmpeg_co...
media/ffmpeg/ffmpeg_common_unittest.cc:17: InitializeMediaLibrary();
Shouldn't be necessary?

[grt (UTC plus 2), 2015-06-02 18:28:05]

chrome/installer/mini_installer/chrome.release and
chrome/tools/build/win/FILES.cfg lgtm

[chcunningham, 2015-06-02 21:44:35]

chcunningham@chromium.org changed reviewers:
+ enne@chromium.org, gunsch@chromium.org, isherman@chromium.org,
jochen@chromium.org

[chcunningham, 2015-06-02 21:44:37]

Adding a few missing owners:

enne@chromium.org - cc/trees/layer_tree_host_*
gunsch@chromium.org - chromecast/media/cma/test/
jochen@chromium.org - content/
isherman@chromium.org - tools/metrics/histograms/

[Ilya Sherman, 2015-06-02 21:49:29]

histograms.xml lgtm

[gunsch, 2015-06-02 21:55:45]

chromecast lgtm

[chcunningham, 2015-06-03 01:14:16]

https://codereview.chromium.org/1141703002/diff/510001/chrome/utility/extensi...
File chrome/utility/extensions/extensions_handler.cc (right):

https://codereview.chromium.org/1141703002/diff/510001/chrome/utility/extensi...
chrome/utility/extensions/extensions_handler.cc:68: // Initialize media
libraries for media file validation.
On 2015/06/02 18:21:06, DaleCurtis wrote:
> Doesn't really need to be pre-sandbox startup anymore; we should find a better
> location for this, but it's fine for now.

SGTM. Filed https://code.google.com/p/chromium/issues/detail?id=495881

https://codereview.chromium.org/1141703002/diff/510001/media/base/media.cc
File media/base/media.cc (right):

https://codereview.chromium.org/1141703002/diff/510001/media/base/media.cc#ne...
media/base/media.cc:23: void Initialize() {
On 2015/06/02 18:21:07, DaleCurtis wrote:
> No need for an Initilize() method and a lock anymore, this can be condensed
into
> the constructor and initialized_ deleted.

Done.

https://codereview.chromium.org/1141703002/diff/510001/media/base/media.cc#ne...
media/base/media.cc:27: // Disable logging as it interferes with layout tests.
On 2015/06/02 18:21:07, DaleCurtis wrote:
> Indent is wrong for these, it should be as if the #if wasn't there.

Done.

https://codereview.chromium.org/1141703002/diff/510001/media/ffmpeg/ffmpeg_co...
File media/ffmpeg/ffmpeg_common_unittest.cc (right):

https://codereview.chromium.org/1141703002/diff/510001/media/ffmpeg/ffmpeg_co...
media/ffmpeg/ffmpeg_common_unittest.cc:17: InitializeMediaLibrary();
On 2015/06/02 18:21:07, DaleCurtis wrote:
> Shouldn't be necessary?

Done.

[chcunningham, 2015-06-03 01:24:29]

chcunningham@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[chcunningham, 2015-06-03 01:24:32]

+jamiewalch@chromium.org for remoting

[jochen (gone - plz use gerrit), 2015-06-03 10:50:40]

lgtm

[Jamie, 2015-06-03 17:02:22]

I'm concerned about the remark "as long as we include the license text in the
credits". remoting_me2me_host has no UI and therefore no credits. Is this a
concern?

[DaleCurtis, 2015-06-03 17:32:23]

I'll follow up with you Jamie.

[enne (OOO), 2015-06-03 18:22:30]

cc lgtm

[Jamie, 2015-06-03 18:48:35]

jamiewalch@chromium.org changed reviewers:
+ sergeyu@chromium.org

[Jamie, 2015-06-03 18:48:38]

+sergeyu for remoting/

[TVL, 2015-06-03 21:19:33]

lgtm

Just covering chrome/tools/build/mac/* while Mark is out.

[Michael Moss, 2015-06-03 21:20:38]

FILES.cfg and linux installers LGTM

[Sergey Ulanov, 2015-06-03 21:47:30]

remoting LGTM

[chcunningham, 2015-06-03 22:24:25]

The CQ bit was checked by chcunningham@chromium.org

[chcunningham, 2015-06-03 22:24:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
phajdan.jr@chromium.org, dalecurtis@chromium.org, grt@chromium.org,
gunsch@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/1141703002/#ps550002
(title: "Rebase and merge fix")

[commit-bot: I haz the power, 2015-06-03 22:25:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1141703002/550002

[commit-bot: I haz the power, 2015-06-03 22:37:38]

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

[commit-bot: I haz the power, 2015-06-03 22:37:42]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[chcunningham, 2015-06-03 22:54:25]

Thanks everyone for the review!

+andrewhayden - FYI: this reduces binary size thanks to dead code elimination in
ffmpeg as a static library. On windows release build, I see a difference of
265.216 KB. FFmpegsumo.dll goes to 0, and chrome_child.dll will grow to include
the static library, but net net, 265.216 less than ffmpegsumo + chrome_child
prior to this change.

[chcunningham, 2015-06-03 23:04:54]

The CQ bit was checked by chcunningham@chromium.org

[chcunningham, 2015-06-03 23:04:55]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
sergeyu@chromium.org, thomasvl@chromium.org, enne@chromium.org,
gunsch@chromium.org, dalecurtis@chromium.org, phajdan.jr@chromium.org,
mmoss@chromium.org, isherman@chromium.org, grt@chromium.org
Link to the patchset: https://codereview.chromium.org/1141703002/#ps600001
(title: "Rebase and merge fixes")

[commit-bot: I haz the power, 2015-06-03 23:07:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1141703002/600001

[commit-bot: I haz the power, 2015-06-03 23:18:57]

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

[commit-bot: I haz the power, 2015-06-03 23:19:02]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sky, 2015-06-03 23:23:50]

ash_unittests.isolate LGTM

[chcunningham, 2015-06-03 23:25:46]

The CQ bit was checked by chcunningham@chromium.org

[commit-bot: I haz the power, 2015-06-03 23:27:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1141703002/600001

[commit-bot: I haz the power, 2015-06-04 00:22:42]

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

[commit-bot: I haz the power, 2015-06-04 00:22:45]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6...)

[chcunningham, 2015-06-04 17:10:48]

The CQ bit was checked by chcunningham@chromium.org

[chcunningham, 2015-06-04 17:10:49]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
sky@chromium.org, sergeyu@chromium.org, thomasvl@chromium.org,
enne@chromium.org, gunsch@chromium.org, dalecurtis@chromium.org,
phajdan.jr@chromium.org, mmoss@chromium.org, isherman@chromium.org,
grt@chromium.org
Link to the patchset: https://codereview.chromium.org/1141703002/#ps620001
(title: "Rebase")

[commit-bot: I haz the power, 2015-06-04 17:12:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1141703002/620001

[commit-bot: I haz the power, 2015-06-04 19:56:07]

Committed patchset #33 (id:620001)

[commit-bot: I haz the power, 2015-06-04 19:57:45]

Patchset 33 (id:??) landed as
https://crrev.com/bfef6d6ba82b172ac0e48c80d9ccf1a1ca0ea5c5
Cr-Commit-Position: refs/heads/master@{#332891}

[scottmg, 2015-06-04 20:26:02]

Awesome!

[DaleCurtis, 2015-06-04 20:37:26]

\o/

[xhwang, 2015-06-04 20:52:30]

Yay!!!

[Will Harris, 2015-06-04 21:10:47]

great success!!!

[Nico, 2015-06-05 01:17:47]

This broke swarming tests on all bots that use component builds:
http://code.google.com/p/chromium/issues/detail?id=496975

[chcunningham, 2015-06-05 17:09:56]

With a much sadness, this was reverted yesterday to unblock the isolate swarming
issue and give me more time to investigate a win7/8 test failure. Hoping to
re-land this afternoon.

[chcunningham, 2015-06-09 00:34:49]

The CQ bit was checked by chcunningham@chromium.org

[chcunningham, 2015-06-09 00:34:50]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
sky@chromium.org, sergeyu@chromium.org, thomasvl@chromium.org,
enne@chromium.org, gunsch@chromium.org, dalecurtis@chromium.org,
phajdan.jr@chromium.org, mmoss@chromium.org, isherman@chromium.org,
grt@chromium.org
Link to the patchset: https://codereview.chromium.org/1141703002/#ps680001
(title: "Roll DEPS to fix cros, rebase.")

[commit-bot: I haz the power, 2015-06-09 00:37:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1141703002/680001

[commit-bot: I haz the power, 2015-06-09 02:09:57]

Committed patchset #36 (id:680001)

[commit-bot: I haz the power, 2015-06-09 02:10:51]

Patchset 36 (id:??) landed as
https://crrev.com/fd11b3c3aa6730ae1f024811b61913cd63f9d39a
Cr-Commit-Position: refs/heads/master@{#333423}

[Lei Zhang, 2015-06-09 03:15:52]

I wish you incorporated r332950 into the second attempt to land this, because
this just closed the tree.

[chcunningham, 2015-06-09 03:29:31]

On 2015/06/09 03:15:52, Lei Zhang wrote:
> I wish you incorporated r332950 into the second attempt to land this, because
> this just closed the tree.

Hey Lei, sorry I missed that. How can I help? The sizes change is ready to go.
I'd submit this again for you, but I'll probably can't TBR it since I'm not a
committer.

[chcunningham, 2015-06-09 20:22:44]

On 2015/06/09 03:29:31, chcunningham wrote:
> On 2015/06/09 03:15:52, Lei Zhang wrote:
> > I wish you incorporated r332950 into the second attempt to land this,
because
> > this just closed the tree.
> 
> Hey Lei, sorry I missed that. How can I help? The sizes change is ready to go.
> I'd submit this again for you, but I'll probably can't TBR it since I'm not a
> committer.

All: Looks like this is finally in! \o/

Lei resubmitted his change (thanks Lei), bots are all green, perf tests look
good. Now to see how it does in the wild :D

Thanks everyone for your reviews and your help!