Installing an image.

I can repro on samus with 8743.13.0. The Flash binary is correct. Also /opt/google/chrome/pepper/pepper-flash.info shows VERSION="23.0.0.162-r1". Which means Chrome gets the version wrong from somewhere else.

Interestingly enough the problem is not on M-55 (at least with minnie 8775.0.0). Which means the regression happened before branching and got fixed just after.

Greg, could this be due to a component updater change?

I haven't made any changes to the component updater yet, especially not back in M-54. Waffles, do you have any ideas?

0_o ... uh

If there a way to confirm if the .163 build is coming from a component update (e.g. the path in advanced - about://plugins)?

Nothing interesting there. I'll try to bisect.

IIUC, the version in chrome://plugins is based on the FLAPPER_VERSION var compiled into the chrome image, but the right-click menu version is based on the true version of the binaries.

My guess is that one of the DEPS rolls rolled the binaries but not the FLAPPER_VERSION?

We don't component-update on ChromeOS, so I don't see how else this could be caused.

Could the component updater for Chrome OS be advertising it's OS to Omaha as Linux?

It seems like to code to load the component wouldn't be present... but was it maybe complete enough at branch point (Aug 25th)?

Even on trunk, Flash isn't registered as a component on Chrome OS (e.g. does not appear in chrome://components), so for the component updater to be involved in this we'd have to have a reallllly weird bug.

I will look into the commit at which the branch was cut and inspect the Flash binaries that are bundled there.

This is a regression introduced in cd59e9b913623e8fec8bbc9d5517610bc1bfc9a7.

chrome://plugins shows two implementations of Flash: one internal-not-yet-present, and one at /opt/google/chrome/pepper/libpepflashplayer.so.

I'm going on leave tomorrow and handing this off to kerrnel@.

To recap what I've found so far:
On ChromeOS, something other than chrome_content_client.cc registers Flash (or there's a bug in that function - but seems unlikely). I think this happens after chrome_content.cc runs and registers the fake-Flash, but am not sure. Three solution ideas:

(1) Flash is registered before chrome_content_client: expand chrome_content_client to check currently-registered plugins, and not add fakeFlash if there's something there.
(2) Flash is registered after chrome_content_client: that registration code can assume fakeFlash is present and always remove it.
(3) #ifdef out fakeFlash registration on ChromeOS. It's not needed until we unbundle Flash on that platform.

I'm on it.

I built and installed M-54 on my chromebook, but it did not reproduce the issue. Yet, enough people have reproduced it that the problem clearly exists.

ihf@, do I need a specific tag? Otherwise I will tweak the flash versions manually to repro this and then proceed to fix.

I used ChromeOS-test-R54-8743.13.0-samus.tar.xz in https://pantheon.corp.google.com/storage/browser/chromeos-releases/canary-channel/samus/8743.13.0/

I have been digging into this and here is what I know so far. 

As far as I can tell, FLAPPER_AVAILABLE is not defined in the Chrome source when built for Chrome OS. This means that the fake plugin registration never happens. In fact, the functions in chrome_content_client.cc don't seem to locate Flash at all on Chrome OS.

It's actually passed on the command line, and that happens here (https://cs.corp.google.com/chromeos_internal/src/platform2/libchromeos-ui/chromeos/ui/chromium_command_builder.cc?q=ppapi-flash-args+package:%5Echromeos_internal$&l=435). It passes in both the version and plugin path, so my best theory so far is that somehow the wrong version got passed in and so it has the right plugin but the wrong version information. 

However, that version seems to come from the .info file, which ihf@ says has the correct version. I will keep trying to reproduce this as I am unlikely to get definitive answers until I have the problem reproduced.

First of all this doesn't repro on minnie, which is strange. But it seems reliable on Samus.

On 8743.7.0 everything is good:
cros flash $DUT xbuddy://remote/samus/R54-8743.7.0
about:flash
Google Chrome	54.0.2840.8 (unknown)
OS	Chrome OS
Flash plugin	23.0.0.141-r1 /opt/google/chrome/pepper/libpepflashplayer.so

Then the problem starts subtly on 8743.8.0 coinciding with -151 binary, which is listed twice:
about:flash
Google Chrome	54.0.2840.10 (unknown)
Flash plugin	23.0.0.151 internal-not-yet-present
Flash plugin	23.0.0.151-r1 /opt/google/chrome/pepper/libpepflashplayer.so (not used)

Then the new Flash binary lands in https://crosland.corp.google.com/log/8743.8.0..8743.9.0
and we get full divergence: now we have the correct 23.0.0.162-r1 binary installed but about:version and about:flash report also incorrect 23.0.0.151.

Google Chrome	54.0.2840.12 (unknown)
OS	Chrome OS
Flash plugin	23.0.0.151 internal-not-yet-present
Flash plugin	23.0.0.162-r1 /opt/google/chrome/pepper/libpepflashplayer.so (not used)

So, problem must be in Chrome
https://chromium.googlesource.com/chromium/src/+log/54.0.2840.8..54.0.2840.10?pretty=fuller&n=10000

indeed we see waffles fake Flash changes integrated there which introduce "internal-not-yet-present". As CrOS component updater isn't shipping on M54 yet we are probably just missing a few ifdefs there and coordination.

But then there is no explanation yet for ARM behavior. To speculate CrOS is the only platform with ARM Flash binaries and component update doesn't know about them yet?

Just for sanity, problem still on newest R54-8743.20.0.

And for even more sanity problem persists on M55, but is hidden as no divergence:

about:flash
Google Chrome	55.0.2852.0 (unknown)
OS	Chrome OS
Flash plugin	23.0.0.162 internal-not-yet-present
Flash plugin	23.0.0.162-r1 /opt/google/chrome/pepper/libpepflashplayer.so (not used)

I still don't think the component updater is involved here. Of more interest, as I suspected, on Samus (the affected platform), flapper_version.h is generated, but it is not generated on ARM.

However, in flapper_version.h we see: #define FLAPPER_VERSION_STRING "23.0.0.162"

I would have expected to see the incorrect 23.0.0.151 version. I will continue to investigate.

I have finally recreated the problem in source. I cannot test on my samus until Monday, but I am very confident this is the problem. I rolled flash player, in my chrome checkout, back to 23.0.0.151.

Sure enough, flapper_version.h is generated with: #define FLAPPER_VERSION_STRING "23.0.0.151"

This causes a fake flash player, version 23.0.0.151, to be produced. However, Chrome on Chrome OS doesn't use the bundled flash player (is that correct ihf@?). So, it gets 23.0.0.162 from the command line, via Chromium launcher, and now the divergence happens. Chrome has registered a non-existent version of flash player.

I think the fix to this is more involved than just #ifdef-ing the code out on Chrome OS, I have several concerns:

#1) Why is flapper_version.h even being generated for Chrome on Chrome OS (intel only) if it doesn't use the bundled flash player? It must think its building for Linux. I think this should be fixed in the build process.
#2) Since Chrome on Chrome OS does not use bundled flash player, a bunch of Chrome OS specific code in chrome_content_client.cc should just be deleted. None of it does  anything as far as I know.

ihf, does this seem reasonable? If so, I will start fixing this all on Monday. Please note that this work also takes priority over the current component update work as I cannot add new Flash registration code to Chrome while the current registration code does not work correctly.

Yes, this makes sense.

For sanity flapper_version.h on samus has
#define FLAPPER_AVAILABLE 1
#define FLAPPER_VERSION_STRING "23.0.0.162"

while on veyron_minnie it contains just these comments.
// If Flapper were available, we'd define two things:
//  - FLAPPER_AVAILABLE (to indicate availability), and
//  - FLAPPER_VERSION_STRING (with the version of the Flapper that's available
//    as a string, e.g., "11.2.123.456").

Might be due to the recent gyp->gn transition, but I don't know where it is generated.

Yes, thanks for the help on this. And to be clear about the one other point, I think the key here is: if the version of flash in the chrome repo happens to match the version in the Chrome OS rootfs, you see two plugins but they have the same version.

When the Chrome repo has a different version, you see two entirely different versions.

 Issue 644400  has been merged into this issue.

Is this P2 and not blocking M54?

As per  Issue 644400 , this issue seems to be introducing performance issue as well. 

Thanks for pointing that out, this should definitely go into M54.

Impacts enrollment performance too in M54. After clicking "Done",almost need to wait for appx 10 sec for Done button to respond.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cf804457306e0facdb59124fa38b5315493e1da7

commit cf804457306e0facdb59124fa38b5315493e1da7
Author: kerrnel <kerrnel@chromium.org>
Date: Mon Sep 19 18:38:10 2016

Do not reference non-existant bundled flash on Chrome OS.

Chrome on Chrome OS is incorrectly building against the Flash player
bundled in third_party/. However, on Chrome OS, flash player is loaded
from the root partition and passed to Chrome via command line. So Chrome
should not be referencing the versions in flapper_version.h. In
addition, this means the bundled flash player loading code is unused.

BUG= 644795 , 644400 

Review-Url: https://codereview.chromium.org/2340643002
Cr-Commit-Position: refs/heads/master@{#419513}

[modify] https://crrev.com/cf804457306e0facdb59124fa38b5315493e1da7/chrome/common/chrome_content_client.cc
[modify] https://crrev.com/cf804457306e0facdb59124fa38b5315493e1da7/third_party/adobe/flash/BUILD.gn

Your change meets the bar and is auto-approved for M54 (branch: 2840)

I verified this in 55 canary. Time to test it on 54 and then commit it to the 54 branch.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/00777c52ac61dba4cd3d9047ede488f337d4a9a5

commit 00777c52ac61dba4cd3d9047ede488f337d4a9a5
Author: Greg Kerr <kerrnel@chromium.org>
Date: Thu Sep 22 22:38:28 2016

Do not reference non-existant bundled flash on Chrome OS.

Chrome on Chrome OS is incorrectly building against the Flash player
bundled in third_party/. However, on Chrome OS, flash player is loaded
from the root partition and passed to Chrome via command line. So Chrome
should not be referencing the versions in flapper_version.h. In
addition, this means the bundled flash player loading code is unused.

BUG= 644795 , 644400 

Review-Url: https://codereview.chromium.org/2340643002
Cr-Commit-Position: refs/heads/master@{#419513}
(cherry picked from commit cf804457306e0facdb59124fa38b5315493e1da7)

Review URL: https://codereview.chromium.org/2366693003 .

Cr-Commit-Position: refs/branch-heads/2840@{#501}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/00777c52ac61dba4cd3d9047ede488f337d4a9a5/chrome/common/chrome_content_client.cc
[modify] https://crrev.com/00777c52ac61dba4cd3d9047ede488f337d4a9a5/third_party/adobe/flash/BUILD.gn

Landed this with git drover. I built the R-54 branch of Chrome OS, and then built a Chrome branded binary and reproduced the problem in 54. I then cherry-picked the fix into Chrome, verified that the CL still solves the problem in 54, and landed it.

Verified in 54.0.2840.39 (beta).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/00777c52ac61dba4cd3d9047ede488f337d4a9a5

commit 00777c52ac61dba4cd3d9047ede488f337d4a9a5
Author: Greg Kerr <kerrnel@chromium.org>
Date: Thu Sep 22 22:38:28 2016

Do not reference non-existant bundled flash on Chrome OS.

Chrome on Chrome OS is incorrectly building against the Flash player
bundled in third_party/. However, on Chrome OS, flash player is loaded
from the root partition and passed to Chrome via command line. So Chrome
should not be referencing the versions in flapper_version.h. In
addition, this means the bundled flash player loading code is unused.

BUG= 644795 , 644400 

Review-Url: https://codereview.chromium.org/2340643002
Cr-Commit-Position: refs/heads/master@{#419513}
(cherry picked from commit cf804457306e0facdb59124fa38b5315493e1da7)

Review URL: https://codereview.chromium.org/2366693003 .

Cr-Commit-Position: refs/branch-heads/2840@{#501}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/00777c52ac61dba4cd3d9047ede488f337d4a9a5/chrome/common/chrome_content_client.cc
[modify] https://crrev.com/00777c52ac61dba4cd3d9047ede488f337d4a9a5/third_party/adobe/flash/BUILD.gn