Kill platform_backend.DidCreateBrowser, platform_backend. DidStartBrowser, platform_backend. WillCloseBrowser hooks in Telemetry |
|||||
Issue descriptionThese hooks is problematic for a few reason: 1) They makes assumption that the logic of creating a browser is explicitly controlled by Telemetry framework, but it's not necessarily True for case like a user story clicking on screen & that happens to launch a browser. 2) It currently try to SetFullPerformanceModeEnabled & unset based on the number of browesrs object that Telemetry is able to keep track of, which make reasoning about life cycle of Telemetry perf test harder. T
,
Dec 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/3572e89d92c2ce8d58ab62ad7fde5e2f628c1475 commit 3572e89d92c2ce8d58ab62ad7fde5e2f628c1475 Author: Ned Nguyen <nednguyen@google.com> Date: Fri Dec 08 11:55:45 2017 Revert "Remove platform_backend's DidStartBrowser, DidCreateBrowser and WillCloseBrowser hooks" This reverts commit 6c03f202337a29da9ac37e88c7e0d3e65a72f9d0. Reason for revert: this may break webgl_conformance_integration_test Original change's description: > Remove platform_backend's DidStartBrowser, DidCreateBrowser and WillCloseBrowser hooks > > The only part of these hooks that create actual effect on perf test is > the calls to SetFullPerformanceModeEnabled in DidCreateBrowser & unset it in > WillCloseBrowser. As we remove those hooks, those calls are moved to > shared_page_state for now (ideally they should be invoked in story_runner so > other shared_state can reuse this, but > the code there is already way to complexed, so I just leave it there for now). > > The life cycle of SetFullPerformanceModeEnabled now is: > > SetFullPerformanceModeEnabled(...) is called before any test is run (before > browser is started) in SharedPageState.__init__ > > SetFullPerformanceModeEnabled(False) is called after all the test has run > in SharedPageState.TearDownState(). > > Compared with before, we run SetFullPerformanceModeEnabled less often & this is > a good optimization as there is no need to set performance mode many times. > > Notes that this also changes relative order of the ClearCaches & > SetFullPerformanceModeEnabled as following: > > Before: > ClearCaches() (in desktop_browser_finder.Create) > SetFullPerformanceModeEnabled(...) (in browser.__init__) > > After: > > SetFullPerformanceModeEnabled(...) (in shared_state.__init__) > ClearCaches() (in desktop_browser_finder.Create, in shared_state.WillRunStory) > > Hence with this CL, the relative order of these two events would be back to what they were before > the regression in crbug/792357 > > Bug: chromium:792860 > Bug: chromium:792357 > Change-Id: Ibeba9ca0af55921514d4f3fecf692c3a740b1aac > Reviewed-on: https://chromium-review.googlesource.com/814214 > Commit-Queue: Ned Nguyen <nednguyen@google.com> > Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org> TBR=perezju@chromium.org,nednguyen@google.com Change-Id: I0dabdb40acfe646d579fb14ae95cf752a140fcbd No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:792860 , chromium:792357 Reviewed-on: https://chromium-review.googlesource.com/816659 Reviewed-by: Ned Nguyen <nednguyen@google.com> Commit-Queue: Ned Nguyen <nednguyen@google.com> [modify] https://crrev.com/3572e89d92c2ce8d58ab62ad7fde5e2f628c1475/telemetry/telemetry/core/platform.py [modify] https://crrev.com/3572e89d92c2ce8d58ab62ad7fde5e2f628c1475/telemetry/telemetry/internal/platform/platform_backend.py [modify] https://crrev.com/3572e89d92c2ce8d58ab62ad7fde5e2f628c1475/telemetry/telemetry/page/shared_page_state.py [modify] https://crrev.com/3572e89d92c2ce8d58ab62ad7fde5e2f628c1475/telemetry/telemetry/internal/browser/browser.py [modify] https://crrev.com/3572e89d92c2ce8d58ab62ad7fde5e2f628c1475/telemetry/telemetry/testing/fakes/__init__.py [modify] https://crrev.com/3572e89d92c2ce8d58ab62ad7fde5e2f628c1475/telemetry/telemetry/internal/browser/browser_unittest.py
,
Dec 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/f9663d1bd9be20b9277dfda0b5b23d6dda6951c9 commit f9663d1bd9be20b9277dfda0b5b23d6dda6951c9 Author: Nghia Nguyen <nednguyen@google.com> Date: Tue Dec 19 12:47:49 2017 Remove platform_backend's DidStartBrowser and WillCloseBrowser hooks SetFullPerformanceModeEnabled(False) is now called after all the test has run in SharedPageState.TearDownState() instead of relying browser objects tracking which is fragile & complex. This is a partial reland of https://chromium-review.googlesource.com/c/catapult/+/814214 Bug: chromium:792860 Bug: chromium:792357 Bug: chromium:793050 Change-Id: I0e4ec50230ccd32c47c34b92cd30edeae7322edf Reviewed-on: https://chromium-review.googlesource.com/833179 Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org> Commit-Queue: Ned Nguyen <nednguyen@google.com> [modify] https://crrev.com/f9663d1bd9be20b9277dfda0b5b23d6dda6951c9/telemetry/telemetry/core/platform.py [modify] https://crrev.com/f9663d1bd9be20b9277dfda0b5b23d6dda6951c9/telemetry/telemetry/internal/platform/platform_backend.py [modify] https://crrev.com/f9663d1bd9be20b9277dfda0b5b23d6dda6951c9/telemetry/telemetry/page/shared_page_state.py [modify] https://crrev.com/f9663d1bd9be20b9277dfda0b5b23d6dda6951c9/telemetry/telemetry/internal/browser/browser.py [modify] https://crrev.com/f9663d1bd9be20b9277dfda0b5b23d6dda6951c9/telemetry/telemetry/testing/fakes/__init__.py [modify] https://crrev.com/f9663d1bd9be20b9277dfda0b5b23d6dda6951c9/telemetry/telemetry/internal/browser/browser_unittest.py
,
Jan 3 2018
,
Jan 3 2018
,
Jan 16
(6 days ago)
,
Jan 16
(6 days ago)
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Dec 7 2017