Unbundle Flash on Linux.

Unbundle Flash on Linux.

TEST=https://docs.google.com/a/google.com/document/d/1P0WslUXMUO9azLmWZ0AtQ-sGExWbHttr_2gQWDXf3MY/pub
BUG=624086
Committed: https://crrev.com/366d572053dd85c8eeda802f423716d9b008fe01
Cr-Commit-Position: refs/heads/master@{#414249}

[waffles, 2016-08-23 23:08:06]

The CQ bit was checked by waffles@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 23:08:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[waffles, 2016-08-23 23:34:50]

waffles@chromium.org changed reviewers:
+ laforge@chromium.org, thestig@chromium.org

[waffles, 2016-08-23 23:34:51]

laforge, thestig, PTAL

This is the Linux equivalent of https://codereview.chromium.org/2273883002/

[Lei Zhang, 2016-08-23 23:40:08]

lgtm

Please test locally if you can build a Chrome-Official build.

[waffles, 2016-08-24 00:33:56]

The CQ bit was checked by waffles@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-24 00:34:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 01:52:55]

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

[commit-bot: I haz the power, 2016-08-24 01:52:56]

Dry run: This issue passed the CQ dry run.

[grt (UTC plus 2), 2016-08-24 08:54:57]

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

[grt (UTC plus 2), 2016-08-24 08:54:58]

I think chrome/BUILD.gn:262 should also change from:

    # dependencies are on chrome.dll. On Windows, these are not bundled.
    if (!is_win) {
      deps += [ "//third_party/adobe/flash:flapper_binaries" ]
    }

to:

   # dependencies are on chrome.dll.
   if (!is_win && (!is_linux || is_chromeos)) {
     deps += [ "//third_party/adobe/flash:flapper_binaries" ]
   }

which i *think* is equivalent to:

   # dependencies are on chrome.dll.
   if (is_chromeos) {
     deps += [ "//third_party/adobe/flash:flapper_binaries" ]
   }

[grt (UTC plus 2), 2016-08-24 08:55:10]

Description was changed from

==========
Unbundle Flash on Linux.

TEST=https://docs.google.com/a/google.com/document/d/1P0WslUXMUO9azLmWZ0AtQ-sGExWbHttr_2gQWDXf3MY/pub
BUG=624086
==========

to

==========
Unbundle Flash on Linux.

TEST=https://docs.google.com/a/google.com/document/d/1P0WslUXMUO9azLmWZ0AtQ-sGExWbHttr_2gQWDXf3MY/pub
BUG=624086
==========

[grt (UTC plus 2), 2016-08-24 08:55:10]

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

[waffles, 2016-08-24 19:28:41]

The CQ bit was checked by waffles@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-24 19:29:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[waffles, 2016-08-24 19:32:05]

On 2016/08/24 08:54:58, grt (UTC plus 2) wrote:
> I think chrome/BUILD.gn:262 should also change from:
> 
>     # dependencies are on chrome.dll. On Windows, these are not bundled.
>     if (!is_win) {
>       deps += [ "//third_party/adobe/flash:flapper_binaries" ]
>     }
> 
> to:
> 
>    # dependencies are on chrome.dll.
>    if (!is_win && (!is_linux || is_chromeos)) {
>      deps += [ "//third_party/adobe/flash:flapper_binaries" ]
>    }
> 
> which i *think* is equivalent to:
> 
>    # dependencies are on chrome.dll.
>    if (is_chromeos) {
>      deps += [ "//third_party/adobe/flash:flapper_binaries" ]
>    }

Done.

> Please test locally if you can build a Chrome-Official build.

Browser tests done. I remembered that there would be a small issue in
chrome_content_client.cc; fixed in the latest patchset; as soon as my build
completes I'll test installing / the behavior of the generated .deb.

[commit-bot: I haz the power, 2016-08-24 21:33:44]

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

[commit-bot: I haz the power, 2016-08-24 21:33:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[waffles, 2016-08-24 23:10:07]

On 2016/08/24 19:32:05, waffles wrote:
> On 2016/08/24 08:54:58, grt (UTC plus 2) wrote:
> > I think chrome/BUILD.gn:262 should also change from:
> > 
> >     # dependencies are on chrome.dll. On Windows, these are not bundled.
> >     if (!is_win) {
> >       deps += [ "//third_party/adobe/flash:flapper_binaries" ]
> >     }
> > 
> > to:
> > 
> >    # dependencies are on chrome.dll.
> >    if (!is_win && (!is_linux || is_chromeos)) {
> >      deps += [ "//third_party/adobe/flash:flapper_binaries" ]
> >    }
> > 
> > which i *think* is equivalent to:
> > 
> >    # dependencies are on chrome.dll.
> >    if (is_chromeos) {
> >      deps += [ "//third_party/adobe/flash:flapper_binaries" ]
> >    }
> 
> Done.
> 
> > Please test locally if you can build a Chrome-Official build.
> 
> Browser tests done. I remembered that there would be a small issue in
> chrome_content_client.cc; fixed in the latest patchset; as soon as my build
> completes I'll test installing / the behavior of the generated .deb.

Things look good with the generated .deb; everything is working as expected.

[waffles, 2016-08-24 23:33:28]

On 2016/08/24 23:10:07, waffles wrote:
> On 2016/08/24 19:32:05, waffles wrote:
> > On 2016/08/24 08:54:58, grt (UTC plus 2) wrote:
> > > I think chrome/BUILD.gn:262 should also change from:
> > > 
> > >     # dependencies are on chrome.dll. On Windows, these are not bundled.
> > >     if (!is_win) {
> > >       deps += [ "//third_party/adobe/flash:flapper_binaries" ]
> > >     }
> > > 
> > > to:
> > > 
> > >    # dependencies are on chrome.dll.
> > >    if (!is_win && (!is_linux || is_chromeos)) {
> > >      deps += [ "//third_party/adobe/flash:flapper_binaries" ]
> > >    }
> > > 
> > > which i *think* is equivalent to:
> > > 
> > >    # dependencies are on chrome.dll.
> > >    if (is_chromeos) {
> > >      deps += [ "//third_party/adobe/flash:flapper_binaries" ]
> > >    }
> > 
> > Done.
> > 
> > > Please test locally if you can build a Chrome-Official build.
> > 
> > Browser tests done. I remembered that there would be a small issue in
> > chrome_content_client.cc; fixed in the latest patchset; as soon as my build
> > completes I'll test installing / the behavior of the generated .deb.
> 
> Things look good with the generated .deb; everything is working as expected.

Lei, do you want to re-review with the two additional files, or should I just go
ahead and CQ?

[Lei Zhang, 2016-08-24 23:42:50]

still lgtm

[waffles, 2016-08-24 23:44:21]

The CQ bit was checked by waffles@chromium.org

[commit-bot: I haz the power, 2016-08-24 23:45:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 02:09:35]

Description was changed from

==========
Unbundle Flash on Linux.

TEST=https://docs.google.com/a/google.com/document/d/1P0WslUXMUO9azLmWZ0AtQ-sGExWbHttr_2gQWDXf3MY/pub
BUG=624086
==========

to

==========
Unbundle Flash on Linux.

TEST=https://docs.google.com/a/google.com/document/d/1P0WslUXMUO9azLmWZ0AtQ-sGExWbHttr_2gQWDXf3MY/pub
BUG=624086
==========

[commit-bot: I haz the power, 2016-08-25 02:09:37]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-25 02:12:51]

Description was changed from

==========
Unbundle Flash on Linux.

TEST=https://docs.google.com/a/google.com/document/d/1P0WslUXMUO9azLmWZ0AtQ-sGExWbHttr_2gQWDXf3MY/pub
BUG=624086
==========

to

==========
Unbundle Flash on Linux.

TEST=https://docs.google.com/a/google.com/document/d/1P0WslUXMUO9azLmWZ0AtQ-sGExWbHttr_2gQWDXf3MY/pub
BUG=624086

Committed: https://crrev.com/366d572053dd85c8eeda802f423716d9b008fe01
Cr-Commit-Position: refs/heads/master@{#414249}
==========

[commit-bot: I haz the power, 2016-08-25 02:12:52]

Patchset 3 (id:??) landed as
https://crrev.com/366d572053dd85c8eeda802f423716d9b008fe01
Cr-Commit-Position: refs/heads/master@{#414249}

[grt (UTC plus 2), 2016-08-25 06:21:40]

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

[grt (UTC plus 2), 2016-08-25 06:21:40]

FYI: I think this will cause an official build break since
chrome/tools/build/win/FILES.cfg still references PepperFlash; see
https://crbug.com/640876 for what happened on Windows. :-( The good news is that
this change didn't make the cut for 54.0.2839.0, so you likely have the day to
fix it.

[waffles, 2016-08-25 16:53:52]

On 2016/08/25 06:21:40, grt (UTC plus 2) wrote:
> FYI: I think this will cause an official build break since
> chrome/tools/build/win/FILES.cfg still references PepperFlash; see
> https://crbug.com/640876 for what happened on Windows. :-( The good news is
that
> this change didn't make the cut for 54.0.2839.0, so you likely have the day to
> fix it.

Ah, thanks; looks like you already took care of it in
https://codereview.chromium.org/2278843002; appreciate it!

[Michael Moss, 2016-08-25 16:57:37]

On 2016/08/25 16:53:52, waffles wrote:
> On 2016/08/25 06:21:40, grt (UTC plus 2) wrote:
> > FYI: I think this will cause an official build break since
> > chrome/tools/build/win/FILES.cfg still references PepperFlash; see
> > https://crbug.com/640876 for what happened on Windows. :-( The good news is
> that
> > this change didn't make the cut for 54.0.2839.0, so you likely have the day
to
> > fix it.
> 
> Ah, thanks; looks like you already took care of it in
> https://codereview.chromium.org/2278843002; appreciate it!

Actually, that was a typo in the comment. For the Linux removal, it needs to be
removed from tools/build/linux/FILES.cfg

[waffles, 2016-08-25 17:31:50]

On 2016/08/25 16:57:37, Michael Moss wrote:
> On 2016/08/25 16:53:52, waffles wrote:
> > On 2016/08/25 06:21:40, grt (UTC plus 2) wrote:
> > > FYI: I think this will cause an official build break since
> > > chrome/tools/build/win/FILES.cfg still references PepperFlash; see
> > > https://crbug.com/640876 for what happened on Windows. :-( The good news
is
> > that
> > > this change didn't make the cut for 54.0.2839.0, so you likely have the
day
> to
> > > fix it.
> > 
> > Ah, thanks; looks like you already took care of it in
> > https://codereview.chromium.org/2278843002; appreciate it!
> 
> Actually, that was a typo in the comment. For the Linux removal, it needs to
be
> removed from tools/build/linux/FILES.cfg

Ok, that makes more sense (I was thinking "but it's already in a !is_win...").
https://codereview.chromium.org/2268403007

[grt (UTC plus 2), 2016-08-26 07:29:49]

On 2016/08/25 16:57:37, Michael Moss wrote:
> On 2016/08/25 16:53:52, waffles wrote:
> > On 2016/08/25 06:21:40, grt (UTC plus 2) wrote:
> > > FYI: I think this will cause an official build break since
> > > chrome/tools/build/win/FILES.cfg still references PepperFlash; see
> > > https://crbug.com/640876 for what happened on Windows. :-( The good news
is
> > that
> > > this change didn't make the cut for 54.0.2839.0, so you likely have the
day
> to
> > > fix it.
> > 
> > Ah, thanks; looks like you already took care of it in
> > https://codereview.chromium.org/2278843002; appreciate it!
> 
> Actually, that was a typo in the comment. For the Linux removal, it needs to
be
> removed from tools/build/linux/FILES.cfg

Oops, yeah. Copy-n-paste then forgot to edit. Sorry 'bout that. Glad you caught
the right thing in the end.