All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=623804

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgwt_7pwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgouLCsAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgopuvpQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgotmrogkM


Bot(s) for this bug's original alert(s):

chromium-rel-mac-retina
chromium-rel-mac11

=== Auto-CCing suspected CL author waffles@chromium.org ===

Hi waffles@chromium.org, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Refactor flash component installer to use DefaultComponentInstaller.
Author  : waffles
Commit description:
  This enables it to use differential updates.

BUG= 601928 

Review-Url: https://codereview.chromium.org/2041573002
Cr-Commit-Position: refs/heads/master@{#401973}
Commit  : 5a2538963cbd28c53ed0f4befc24bd9b4d99f77c
Date    : Fri Jun 24 21:24:53 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@401950  1092.09  5.72117  5  good
chromium@401963  1089.94  2.72737  5  good
chromium@401970  1095.2   7.2807   5  good
chromium@401972  1089.48  3.17721  5  good
chromium@401973  1136.43  3.55652  5  bad    <--
chromium@401976  1140.21  7.68748  5  bad

Bisect job ran on: mac_10_11_perf_bisect
Bug ID: 623804

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests page_cycler.typical_25
Test Metric: warm_times/page_load_time
Relative Change: 4.41%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_10_11_perf_bisect/builds/697
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9008650224391778240


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5807847586136064

| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Tests>AutoBisect.  Thank you!

Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I'm confused. The code changed in 2041573002 doesn't even compile for Chromium builds. The whole thing is in #if defined(GOOGLE_CHROME_BUILD).

chromium-rel-mac-retina and chromium-rel-mac11 are Chromium builds, right? (Not Google Chrome builds?)

I asked the Infra team, actually your change affects these chromium bots, this define is for Chrome-branded builds.

Wait, so the chromium-rel bots build Chrome-branded builds? (That's very confusing.)

Can you tell me about what page_cycler.typical_25 is doing?

It's possible for this change to cause more I/O during browser start-up for Chrome-branded builds, as we will now read a file to determine the version of Flash instead of relying on compiled-in code. (In practice, this startup regression already exists for the common case of component-updated Flash, but that would certainly escape our testing, since it's a condition that develops on users' machines over time.)

kouhei@, nednguyen@, can you please take a look at this patch and see if it causes a regression on the page_cycler.typical_25 test?

Looks like the alert is recovered? 
page_cycler.typical_25 cycles through set of URLs and measure their page-load-times and related metrics.

OK, so it looks like these alerts recovered with https://chromium.googlesource.com/chromium/src/+/4d66c388052c2c22791e859e6918e779fa5e4732

which actually has a side effect of breaking the loading of Flash from the disk.

The performance regressions recur after the code is fixed with https://chromium.googlesource.com/chromium/src/+/3436cd4cbccb4c573185e2faefc248727e1baecb.

The fact that 3436cd4cbccb4c573185e2faefc248727e1baecb caused the regression  to recur seems like strong evidence that the performance hit is either:

(A) under PepperFlashComponentInstaller::RegisterPepperFlashWithChrome (https://cs.chromium.org/chromium/src/chrome/browser/component_updater/pepper_flash_component_installer.cc&l=111) 
(B) due to having a working flash plugin registered

I'm leaning toward A because presumably these tests had working Flash implementations before (unless the bots build with !ENABLE_PLUGINS, or I'm missing something else).

Reading the code carefully, I do see that we can unregister then re-register the same version of Flash (which will result in some IPC due to PluginListChanged), I'm landing a speculative patch to see if removing this execution path will fix the regression.

 Issue 629262  has been merged into this issue.

Hey, does anyone know if each bisect execution uses a hermetic DIR_USER_DATA?

If DIR_USER_DATA has certain contents that might explain what is going on here.

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

commit c3ad72bca3073c1a89c7bcf6f191015b7aaddc0f
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Tue Jul 26 19:47:44 2016

Roll src/third_party/catapult/ 892d222cd..25b9a024b (2 commits).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/892d222cd7ed..25b9a024bb5d

$ git log 892d222cd..25b9a024b --date=short --no-merges --format='%ad %ae %s'

BUG= 623804 

TBR=catapult-sheriff@chromium.org

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

[modify] https://crrev.com/c3ad72bca3073c1a89c7bcf6f191015b7aaddc0f/DEPS

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

commit 1eadbdccdb0dee6fe9e169ac374bfa6c1ad9d03d
Author: waffles <waffles@chromium.org>
Date: Fri Jul 29 22:41:35 2016

Speculative fix for perf test performance regressions.

BUG= 623804 

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

[modify] https://crrev.com/1eadbdccdb0dee6fe9e169ac374bfa6c1ad9d03d/chrome/browser/component_updater/pepper_flash_component_installer.cc

 Issue 629296  has been merged into this issue.

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

commit 01bd53dba19f1df9dd002a15c87a1ea180c1ffbb
Author: guidou <guidou@chromium.org>
Date: Mon Aug 01 14:10:41 2016

Revert of Speculative fix for perf test performance regressions. (patchset #1 id:1 of https://codereview.chromium.org/2190083002/ )

Reason for revert:
Speculative revert to see if that fixes the WebRTC Win7 Tester bot.

See https://build.chromium.org/p/chromium.webrtc/builders/Win7%20Tester/builds/28171

Sample error logs:
[ RUN      ] PPAPINaClPNaClTest.MediaStreamVideoTrack
[4788:5076:0729/174935:WARNING:chrome_browser_main_win.cc(419)] Command line too long for RegisterApplicationRestart
[5744:3076:0729/174935:ERROR:dxva_video_decode_accelerator_win.cc(273)] EGL_EXT_device_query missing
[5744:3076:0729/174935:ERROR:mf_helpers.cc(12)] Error in dxva_video_decode_accelerator_win.cc on line 273
[6048:3772:0729/174935:ERROR:singleton_hwnd.cc(34)] Cannot create windows on non-UI thread!
[4788:4472:0729/174936:WARNING:embedded_test_server.cc(202)] Request not handled. Returning 404: /favicon.ico
[4788:4168:0729/174936:ERROR:nacl_process_host.cc(337)] NaCl process exited with status -1073741674 (0xc0000096)
[4788:5076:0729/174936:INFO:CONSOLE(0)] "NativeClient: NaCl module crashed", source: http://127.0.0.1:50824/test_case.html?mode=nacl_pnacl&testcase=MediaStreamVideoTrack (0)
e:\b\c\b\win_builder\src\chrome\test\ppapi\ppapi_test.cc(249): error: Value of: handler.message().c_str()
  Actual: "Plugin crashed. 'NaCl module crashed'"
Expected: "PASS"
[5744:3300:0729/174936:ERROR:node_controller.cc(1099)] Could not be introduced to peer 71198AAF592CFFC1.CFEE94F16E49B4BD
[4788:5076:0729/174937:WARNING:url_request_context_getter.cc(43)] URLRequestContextGetter leaking due to no owning thread.
[  FAILED  ] PPAPINaClPNaClTest.MediaStreamVideoTrack, where TypeParam =  and GetParam() =  (2255 ms)
[98/98] PPAPINaClPNaClTest.MediaStreamVideoTrack (2556 ms)
Retrying 1 test (retry #3)
[ RUN      ] PPAPINaClPNaClTest.MediaStreamVideoTrack
[5628:4292:0729/174937:WARNING:chrome_browser_main_win.cc(419)] Command line too long for RegisterApplicationRestart
[1440:4176:0729/174937:ERROR:dxva_video_decode_accelerator_win.cc(273)] EGL_EXT_device_query missing
[1440:4176:0729/174937:ERROR:mf_helpers.cc(12)] Error in dxva_video_decode_accelerator_win.cc on line 273
[4792:5844:0729/174937:ERROR:singleton_hwnd.cc(34)] Cannot create windows on non-UI thread!
[5628:6016:0729/174938:WARNING:embedded_test_server.cc(202)] Request not handled. Returning 404: /favicon.ico
[5628:6104:0729/174939:ERROR:nacl_process_host.cc(337)] NaCl process exited with status -1073741674 (0xc0000096)
[5628:4292:0729/174939:INFO:CONSOLE(0)] "NativeClient: NaCl module crashed", source: http://127.0.0.1:50830/test_case.html?mode=nacl_pnacl&testcase=MediaStreamVideoTrack (0)
e:\b\c\b\win_builder\src\chrome\test\ppapi\ppapi_test.cc(249): error: Value of: handler.message().c_str()
  Actual: "Plugin crashed. 'NaCl module crashed'"
Expected: "PASS"
[1440:1012:0729/174939:ERROR:node_controller.cc(1099)] Could not be introduced to peer 326301B2EC2EF45.56A4474DAFD5DD29
[5628:4292:0729/174939:WARNING:url_request_context_getter.cc(43)] URLRequestContextGetter leaking due to no owning thread.
[  FAILED  ] PPAPINaClPNaClTest.MediaStreamVideoTrack, where TypeParam =  and GetParam() =  (2243 ms)
[99/99] PPAPINaClPNaClTest.MediaStreamVideoTrack (2537 ms)
1 test failed:
    PPAPINaClPNaClTest.MediaStreamVideoTrack (e:\b\c\b\win_builder\src\chrome\test\ppapi\ppapi_browsertest.cc:1164)

Will reland if the revert doesn't fix the issue.

Original issue's description:
> Speculative fix for perf test performance regressions.
>
> BUG= 623804 
>
> Committed: https://crrev.com/1eadbdccdb0dee6fe9e169ac374bfa6c1ad9d03d
> Cr-Commit-Position: refs/heads/master@{#408789}

TBR=wfh@chromium.org,waffles@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 623804 

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

[modify] https://crrev.com/01bd53dba19f1df9dd002a15c87a1ea180c1ffbb/chrome/browser/component_updater/pepper_flash_component_installer.cc

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

commit 34f936894ac029b47f61b82c620567743fc7eca9
Author: guidou <guidou@chromium.org>
Date: Mon Aug 01 15:38:52 2016

Reland of Speculative fix for perf test performance regressions. (patchset #1 id:1 of https://codereview.chromium.org/2196103003/ )

Reason for revert:
The revert did not fix the issue it was suspect of causing.

Original issue's description:
> Revert of Speculative fix for perf test performance regressions. (patchset #1 id:1 of https://codereview.chromium.org/2190083002/ )
>
> Reason for revert:
> Speculative revert to see if that fixes the WebRTC Win7 Tester bot.
>
> See https://build.chromium.org/p/chromium.webrtc/builders/Win7%20Tester/builds/28171
>
> Sample error logs:
> [ RUN      ] PPAPINaClPNaClTest.MediaStreamVideoTrack
> [4788:5076:0729/174935:WARNING:chrome_browser_main_win.cc(419)] Command line too long for RegisterApplicationRestart
> [5744:3076:0729/174935:ERROR:dxva_video_decode_accelerator_win.cc(273)] EGL_EXT_device_query missing
> [5744:3076:0729/174935:ERROR:mf_helpers.cc(12)] Error in dxva_video_decode_accelerator_win.cc on line 273
> [6048:3772:0729/174935:ERROR:singleton_hwnd.cc(34)] Cannot create windows on non-UI thread!
> [4788:4472:0729/174936:WARNING:embedded_test_server.cc(202)] Request not handled. Returning 404: /favicon.ico
> [4788:4168:0729/174936:ERROR:nacl_process_host.cc(337)] NaCl process exited with status -1073741674 (0xc0000096)
> [4788:5076:0729/174936:INFO:CONSOLE(0)] "NativeClient: NaCl module crashed", source: http://127.0.0.1:50824/test_case.html?mode=nacl_pnacl&testcase=MediaStreamVideoTrack (0)
> e:\b\c\b\win_builder\src\chrome\test\ppapi\ppapi_test.cc(249): error: Value of: handler.message().c_str()
>   Actual: "Plugin crashed. 'NaCl module crashed'"
> Expected: "PASS"
> [5744:3300:0729/174936:ERROR:node_controller.cc(1099)] Could not be introduced to peer 71198AAF592CFFC1.CFEE94F16E49B4BD
> [4788:5076:0729/174937:WARNING:url_request_context_getter.cc(43)] URLRequestContextGetter leaking due to no owning thread.
> [  FAILED  ] PPAPINaClPNaClTest.MediaStreamVideoTrack, where TypeParam =  and GetParam() =  (2255 ms)
> [98/98] PPAPINaClPNaClTest.MediaStreamVideoTrack (2556 ms)
> Retrying 1 test (retry #3)
> [ RUN      ] PPAPINaClPNaClTest.MediaStreamVideoTrack
> [5628:4292:0729/174937:WARNING:chrome_browser_main_win.cc(419)] Command line too long for RegisterApplicationRestart
> [1440:4176:0729/174937:ERROR:dxva_video_decode_accelerator_win.cc(273)] EGL_EXT_device_query missing
> [1440:4176:0729/174937:ERROR:mf_helpers.cc(12)] Error in dxva_video_decode_accelerator_win.cc on line 273
> [4792:5844:0729/174937:ERROR:singleton_hwnd.cc(34)] Cannot create windows on non-UI thread!
> [5628:6016:0729/174938:WARNING:embedded_test_server.cc(202)] Request not handled. Returning 404: /favicon.ico
> [5628:6104:0729/174939:ERROR:nacl_process_host.cc(337)] NaCl process exited with status -1073741674 (0xc0000096)
> [5628:4292:0729/174939:INFO:CONSOLE(0)] "NativeClient: NaCl module crashed", source: http://127.0.0.1:50830/test_case.html?mode=nacl_pnacl&testcase=MediaStreamVideoTrack (0)
> e:\b\c\b\win_builder\src\chrome\test\ppapi\ppapi_test.cc(249): error: Value of: handler.message().c_str()
>   Actual: "Plugin crashed. 'NaCl module crashed'"
> Expected: "PASS"
> [1440:1012:0729/174939:ERROR:node_controller.cc(1099)] Could not be introduced to peer 326301B2EC2EF45.56A4474DAFD5DD29
> [5628:4292:0729/174939:WARNING:url_request_context_getter.cc(43)] URLRequestContextGetter leaking due to no owning thread.
> [  FAILED  ] PPAPINaClPNaClTest.MediaStreamVideoTrack, where TypeParam =  and GetParam() =  (2243 ms)
> [99/99] PPAPINaClPNaClTest.MediaStreamVideoTrack (2537 ms)
> 1 test failed:
>     PPAPINaClPNaClTest.MediaStreamVideoTrack (e:\b\c\b\win_builder\src\chrome\test\ppapi\ppapi_browsertest.cc:1164)
>
> Will reland if the revert doesn't fix the issue.
>
> Original issue's description:
> > Speculative fix for perf test performance regressions.
> >
> > BUG= 623804 
> >
> > Committed: https://crrev.com/1eadbdccdb0dee6fe9e169ac374bfa6c1ad9d03d
> > Cr-Commit-Position: refs/heads/master@{#408789}
>
> TBR=wfh@chromium.org,waffles@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= 623804 
>
> Committed: https://crrev.com/01bd53dba19f1df9dd002a15c87a1ea180c1ffbb
> Cr-Commit-Position: refs/heads/master@{#408956}

TBR=wfh@chromium.org,waffles@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 623804 

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

[modify] https://crrev.com/34f936894ac029b47f61b82c620567743fc7eca9/chrome/browser/component_updater/pepper_flash_component_installer.cc

OK, so the root cause is that Flash is now running on these pages, whereas before it wasn't:

[Status quo]: The perf test framework executes Chrome with --ppapi-flash-path=C:\b\c\b\Win_10_Perf__3_\src\third_party\adobe\flash\binaries\ppapi\win_x64\pepflashplayer.dll. But, because it doesn't pass --ppapi-flash-version, Chrome assumes the version of Flash here is 10.2.999.999. This is old enough that the browser will not run it, instead displaying a placeholder stating "Flash is out of date".

401973: [Regression appears] Refactoring the Flash installer allows the component updater to see the bundled version of Flash and surmise that the version here is 22.0.0.209; this is more up-to-date than 10.2.999.999 and so it overrides the flag-specified version and Flash is now running on these pages.

403476: [Regression disappears] This change breaks the component-installer loaded Flash; the plugin can't be found by the renderers so they don't render the Flash content.

405419: [Regression reappears] This change fixes component-installer loaded Flash; Flash begins running on these pages again.

catapult:db2aa9: Thinking the issue is caused by the path by which Flash is loaded, I commit this fix to cause the flag value specified by the perf test framework to not be overridden. The pages continue to load Flash, again from the flag path, but without the assumption that it is 10.2.999.999; therefore it loads the content instead of displaying the placeholder.

408789: This fully disables component-updater-loaded Flash (bundled or component-updated) in the presence of --ppapi-flash-path. But, it's ineffective because of db2aa9, committed above: we're already loading Flash from the flag path.

[Current Status] The perf test framework executes Chrome with --ppapi-flash-path=C:\b\c\b\Win_10_Perf__3_\src\third_party\adobe\flash\binaries\ppapi\win_x64\pepflashplayer.dll. But now it also passes --ppapi-flash-version, which is a high enough value that Chrome will run the plugin instead of showing the placeholder.


So now I see two options:
(1) Return to status quo: Do not include Flash in our perf testing. In this case, we might as well simplify the perf testing framework and eliminate all the logic to copy Flash executables around / specify --ppapi-flash-path, because under the status quo, there was no point in doing all this anyways. (Since the browser will just refuse to run the supposedly-old plugin.) (We might as well just pass kDisableBundledPpapiFlash.)
(2) Close the perf-regression bug as working-as-intended. We are now testing these pages as the users will see them / as they are meant to be displayed (with the presence of Flash). Our historical results were essentially false signals, because we were testing without Flash.

Looking for guidance from you all on which of the above to select, I can see pros and cons for both options.

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

commit 95b8b85ada46836423ff89e97706369981b6d271
Author: waffles <waffles@chromium.org>
Date: Mon Aug 01 18:50:43 2016

Revert of Speculative fix for perf test performance regressions. (patchset #1 id:1 of https://codereview.chromium.org/2190083002/ )

Reason for revert:
This patch has no effect on perf test performance regressions, although it helped me understand what is really going on.

I've identified two paths forward and neither of them involves having this behavior; hence, removing it.

Original issue's description:
> Speculative fix for perf test performance regressions.
>
> BUG= 623804 
>
> Committed: https://crrev.com/1eadbdccdb0dee6fe9e169ac374bfa6c1ad9d03d
> Cr-Commit-Position: refs/heads/master@{#408789}

TBR=wfh@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 623804 

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

[modify] https://crrev.com/95b8b85ada46836423ff89e97706369981b6d271/chrome/browser/component_updater/pepper_flash_component_installer.cc

I am closing this as working-as-intended. (This is taking option (2) as called out in #18.) Obviously, please respond if you have any objections.

 Issue 629308  has been merged into this issue.