New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 796940 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: ----



Sign in to add a comment

webkit_unit_tests failing on chromium.webkit/WebKit Android (Nexus4)

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Dec 21 2017

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of markusheintz@chromium.org

webkit_unit_tests failing on chromium.webkit/WebKit Android (Nexus4)

Builders failed on: 
- WebKit Android (Nexus4): 
  https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Android%20%28Nexus4%29


 
Cc: haraken@chromium.org rogerm@chromium.org tikuta@chromium.org osh...@chromium.org
Owner: fmea...@chromium.org
Started reliable failing at https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Android%20%28Nexus4%29/72514

Suspects:
- https://chromium-review.googlesource.com/c/chromium/src/+/836627
- https://chromium-review.googlesource.com/c/chromium/src/+/815678
Speculatively reverted my CL in 
https://chromium-review.googlesource.com/c/chromium/src/+/840160
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 21 2017

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

commit e442ea0666d7af6e564fc1eb2c31deb05ed54035
Author: Fadi Meawad <fmeawad@chromium.org>
Date: Thu Dec 21 22:54:43 2017

Reland "[PageLifecyle] Move the launch process state to Webkit/common"

This is a reland of 99a51fcacaad639093357727542fb1ef7c376a25

The failures described in  crbug.com/796940  was result of throttling
the scheduled tasks as a result of the new backgrounding state.

The fix foregrounds the RendererScheduler before running the tests
to avoid throttling.

Original change's description:
> [PageLifecyle] Move the launch process state to Webkit/common
>
> This CL introduces Webkit/common/page/launching_process_state.h that
> includes the launch background and launch boosted state.
>
> The reason this file is needed, is because the state is different per
> OS but needs to be consistent between the RenderProcessHostImpl and
> the RendererSchedulerImpl. To prevent future changes in one not
> reflecting in the other, the code is moved to a common place and
> shared.
>
> This CL fixes an issue where the scheduler assumes that it is
> foregrounded while it is actually backgrounded during the initial
> phase on Android.
>
> Bug:  chromium:780191 
> Change-Id: Ib8dbfb6447f58c8b998a94ae2c69f8bc906d150f
> Reviewed-on: https://chromium-review.googlesource.com/815678
> Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> Reviewed-by: Alexander Timin <altimin@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Bo <boliu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#525563}

TBR=haraken@chromium.org,boliu@chromium.org

Bug:  chromium:780191 , chromium:796940 
Change-Id: I117afad3b910239656dbc7e9dc3a1447291263a9
Reviewed-on: https://chromium-review.googlesource.com/840340
Reviewed-by: Fadi Meawad <fmeawad@chromium.org>
Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525841}
[modify] https://crrev.com/e442ea0666d7af6e564fc1eb2c31deb05ed54035/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/e442ea0666d7af6e564fc1eb2c31deb05ed54035/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/e442ea0666d7af6e564fc1eb2c31deb05ed54035/third_party/WebKit/Source/platform/scheduler/renderer/queueing_time_estimator.h
[modify] https://crrev.com/e442ea0666d7af6e564fc1eb2c31deb05ed54035/third_party/WebKit/Source/platform/scheduler/renderer/queueing_time_estimator_unittest.cc
[modify] https://crrev.com/e442ea0666d7af6e564fc1eb2c31deb05ed54035/third_party/WebKit/Source/platform/scheduler/renderer/renderer_metrics_helper_unittest.cc
[modify] https://crrev.com/e442ea0666d7af6e564fc1eb2c31deb05ed54035/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/e442ea0666d7af6e564fc1eb2c31deb05ed54035/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl_unittest.cc
[modify] https://crrev.com/e442ea0666d7af6e564fc1eb2c31deb05ed54035/third_party/WebKit/common/BUILD.gn
[add] https://crrev.com/e442ea0666d7af6e564fc1eb2c31deb05ed54035/third_party/WebKit/common/page/launching_process_state.h

Status: Fixed (was: Available)
I disabled throttling on the test, and relanded the CL. The failure did not persist after the reland.

Marking as fixed.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 3 2018

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

commit b85b48382bd9087a89afdc6fe0ac33dd71f1487e
Author: Fadi Meawad <fmeawad@chromium.org>
Date: Wed Jan 03 02:18:33 2018

Revert "Reland "[PageLifecyle] Move the launch process state to Webkit/common""

This reverts commit e442ea0666d7af6e564fc1eb2c31deb05ed54035.

Reason for revert: 
This CL caused major regressions in android webview performance.
See  https://crbug.com/798310  

Original change's description:
> Reland "[PageLifecyle] Move the launch process state to Webkit/common"
> 
> This is a reland of 99a51fcacaad639093357727542fb1ef7c376a25
> 
> The failures described in  crbug.com/796940  was result of throttling
> the scheduled tasks as a result of the new backgrounding state.
> 
> The fix foregrounds the RendererScheduler before running the tests
> to avoid throttling.
> 
> Original change's description:
> > [PageLifecyle] Move the launch process state to Webkit/common
> >
> > This CL introduces Webkit/common/page/launching_process_state.h that
> > includes the launch background and launch boosted state.
> >
> > The reason this file is needed, is because the state is different per
> > OS but needs to be consistent between the RenderProcessHostImpl and
> > the RendererSchedulerImpl. To prevent future changes in one not
> > reflecting in the other, the code is moved to a common place and
> > shared.
> >
> > This CL fixes an issue where the scheduler assumes that it is
> > foregrounded while it is actually backgrounded during the initial
> > phase on Android.
> >
> > Bug:  chromium:780191 
> > Change-Id: Ib8dbfb6447f58c8b998a94ae2c69f8bc906d150f
> > Reviewed-on: https://chromium-review.googlesource.com/815678
> > Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> > Reviewed-by: Alexander Timin <altimin@chromium.org>
> > Reviewed-by: Kentaro Hara <haraken@chromium.org>
> > Reviewed-by: Bo <boliu@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#525563}
> 
> TBR=haraken@chromium.org,boliu@chromium.org
> 
> Bug:  chromium:780191 , chromium:796940 
> Change-Id: I117afad3b910239656dbc7e9dc3a1447291263a9
> Reviewed-on: https://chromium-review.googlesource.com/840340
> Reviewed-by: Fadi Meawad <fmeawad@chromium.org>
> Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#525841}

TBR=boliu@chromium.org,fmeawad@chromium.org,haraken@chromium.org,oysteine@chromium.org,altimin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  chromium:780191 ,  chromium:796940 ,  chromium:798310 
Change-Id: I0c8b7e075c29761d2a6f0d02461551b651682748
Reviewed-on: https://chromium-review.googlesource.com/847653
Reviewed-by: Fadi Meawad <fmeawad@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526586}
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/platform/scheduler/renderer/queueing_time_estimator.h
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/platform/scheduler/renderer/queueing_time_estimator_unittest.cc
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/platform/scheduler/renderer/renderer_metrics_helper_unittest.cc
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl_unittest.cc
[modify] https://crrev.com/b85b48382bd9087a89afdc6fe0ac33dd71f1487e/third_party/WebKit/common/BUILD.gn
[delete] https://crrev.com/8d5d19d557008d887d4ea133d9b80594908a0e3a/third_party/WebKit/common/page/launching_process_state.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 18 2018

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

commit bb30dbdbdf9a183ada3e129ffee817b8a6fd494a
Author: Fadi Meawad <fmeawad@chromium.org>
Date: Thu Jan 18 22:04:33 2018

Reland(2): [PageLifecyle] Move the launch process state to Webkit/common

The first reland caused a major regression in Android Webview bots
running with a single process, since Webview had a bug that did not
propagate the background state properly to the renderer scheduler.

This bug was fixed in:
https://chromium-review.googlesource.com/c/chromium/src/+/865769

> Original change's description:
> > Reland "[PageLifecyle] Move the launch process state to Webkit/common"
> >
> > This is a reland of 99a51fcacaad639093357727542fb1ef7c376a25
> >
> > The failures described in  crbug.com/796940  was result of throttling
> > the scheduled tasks as a result of the new backgrounding state.
> >
> > The fix foregrounds the RendererScheduler before running the tests
> > to avoid throttling.
> >
> > Original change's description:
> > > [PageLifecyle] Move the launch process state to Webkit/common
> > >
> > > This CL introduces Webkit/common/page/launching_process_state.h that
> > > includes the launch background and launch boosted state.
> > >
> > > The reason this file is needed, is because the state is different per
> > > OS but needs to be consistent between the RenderProcessHostImpl and
> > > the RendererSchedulerImpl. To prevent future changes in one not
> > > reflecting in the other, the code is moved to a common place and
> > > shared.
> > >
> > > This CL fixes an issue where the scheduler assumes that it is
> > > foregrounded while it is actually backgrounded during the initial
> > > phase on Android.
> > >
> > > Bug:  chromium:780191 
> > > Change-Id: Ib8dbfb6447f58c8b998a94ae2c69f8bc906d150f
> > > Reviewed-on: https://chromium-review.googlesource.com/815678
> > > Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> > > Reviewed-by: Alexander Timin <altimin@chromium.org>
> > > Reviewed-by: Kentaro Hara <haraken@chromium.org>
> > > Reviewed-by: Bo <boliu@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#525563}
> >
> > TBR=haraken@chromium.org,boliu@chromium.org
> >
> > Bug:  chromium:780191 , chromium:796940 
> > Change-Id: I117afad3b910239656dbc7e9dc3a1447291263a9
> > Reviewed-on: https://chromium-review.googlesource.com/840340
> > Reviewed-by: Fadi Meawad <fmeawad@chromium.org>
> > Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#525841}
>
> TBR=boliu@chromium.org,fmeawad@chromium.org,haraken@chromium.org,oysteine@chromium.org,altimin@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug:  chromium:780191 ,  chromium:796940 ,  chromium:798310 
> Change-Id: I0c8b7e075c29761d2a6f0d02461551b651682748
> Reviewed-on: https://chromium-review.googlesource.com/847653
> Reviewed-by: Fadi Meawad <fmeawad@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#526586}

TBR=boliu@chromium.org,altimin@chromium.org

Bug:  chromium:780191 ,  chromium:796940 ,  chromium:798310 
Change-Id: I213457c16ee09f272c5e188bc88e915e23ed40ea
Reviewed-on: https://chromium-review.googlesource.com/871370
Commit-Queue: Fadi Meawad <fmeawad@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530289}
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/platform/scheduler/renderer/queueing_time_estimator.h
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/platform/scheduler/renderer/queueing_time_estimator_unittest.cc
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_metrics_helper_unittest.cc
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl_unittest.cc
[modify] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/common/BUILD.gn
[add] https://crrev.com/bb30dbdbdf9a183ada3e129ffee817b8a6fd494a/third_party/WebKit/common/page/launching_process_state.h

Sign in to add a comment