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

Issue 610338 link

Starred by 1 user

Issue metadata

Status: Fixed
Merged: issue 609941
Owner:
Closed: May 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression



Sign in to add a comment

v8.key_mobile_sites_smooth and smoothness.key_mobile_sites_smooth failure on Android at 391672 - 391710

Project Member Reported by oysteine@chromium.org, May 9 2016

Issue description

Cc: hpayer@chromium.org
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, May 10 2016

Mergedinto: 609941
Status: Duplicate (was: Untriaged)

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


===== SUSPECTED CL(s) =====
Subject : [Reland 1] Pepper takes ownership of a mailbox before passing it to the texture layer.
Author  : erikchen
Commit description:
  
This CL makes three changes from the original.

1. Replace a call to std::remove_if() with vec.erase(std::remove_if(), ...).
This was a logic error in the original CL that caused a crash any time the size
of the buffer was changed. This CL also adds a test that catches this bug.

2. Add some simple reference counting to PepperPluginInstanceImpl to track the
fact that a cc::TextureMailbox may be passed to |texture_layer_| more than once.

3. The SyncToken signal is now processed in the context of its own message:
WaitSyncToken.

> I replaced the IPC message GpuCommandBufferMsg_ProduceFrontBuffer with
> GpuCommandBufferMsg_TakeFrontBuffer and GpuCommandBufferMsg_ReturnFrontBuffer.
> TakeFrontBuffer gives ownership of the front buffer to the client. When the
> client returns it with ReturnFrontBuffer, the command buffer may choose to reuse
> it.
>
> This means that pepper no longer needs to use
> SetTextureMailboxWithoutReleaseCallback.

BUG= 350204 ,  602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
TEST=Run Chromium with the Pepper Flash plugin. Visit a site that supports Flash
video, such as http://vudu.com. Start playing a video, and then fullscreen the
video. Observe that Chromium does not crash. Please extensively test Chromium on
Flash 3D games and Flash video and make sure nothing else is working
incorrectly.
TBR=ccameron@chromium.org, bbudge@chromium.org, sky@chromium.org

Review-Url: https://codereview.chromium.org/1943513002
Cr-Commit-Position: refs/heads/master@{#391686}
Commit  : 4734ed1b6740b772201b8accb3e2bd88cdea5c99
Date    : Wed May 04 23:25:19 2016


===== TESTED REVISIONS =====
Revision         Exit Code  Std Dev  N   Good?
chromium@391672  0          N/A      10  good
chromium@391682  0          N/A      10  good
chromium@391685  0          N/A      10  good
chromium@391686  1          N/A      10  bad    <--
chromium@391687  1          N/A      10  bad
chromium@391691  1          N/A      10  bad
chromium@391710  1          N/A      10  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 610338

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests v8.key_mobile_sites_smooth
Test Metric: cpu_time/cpu_time
Relative Change: Zero to non-zero
Score: 99.8

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3679
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9013118166854757888


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

| 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!
Owner: oysteine@chromium.org
Status: Assigned (was: Duplicate)
Oystein, looks like the culprit CL has been reverted, can you re-enable the test?
Project Member

Comment 5 by bugdroid1@chromium.org, May 10 2016

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

commit 13a76e93830f9ffe6fd03074d01daf00641cc336
Author: oysteine <oysteine@chromium.org>
Date: Tue May 10 14:55:57 2016

Disable smoothness/v8.key_mobile_sites_smooth

TBR=nednguyen
BUG= 610338 

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

[modify] https://crrev.com/13a76e93830f9ffe6fd03074d01daf00641cc336/tools/perf/benchmarks/smoothness.py
[modify] https://crrev.com/13a76e93830f9ffe6fd03074d01daf00641cc336/tools/perf/benchmarks/v8.py

Oops, landed before seeing the last comment.

Enabling again: https://codereview.chromium.org/1966573003/
Project Member

Comment 7 by bugdroid1@chromium.org, May 10 2016

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

commit 527af54566e2435c62dc6a7acf3907e14f7c17af
Author: oysteine <oysteine@chromium.org>
Date: Tue May 10 15:09:19 2016

Revert of Disable smoothness/v8.key_mobile_sites_smooth (patchset #1 id:1 of https://codereview.chromium.org/1956413002/ )

Reason for revert:
Test should be fixed again.

Original issue's description:
> Disable smoothness/v8.key_mobile_sites_smooth
>
> TBR=nednguyen
> BUG= 610338 
>
> Committed: https://crrev.com/13a76e93830f9ffe6fd03074d01daf00641cc336
> Cr-Commit-Position: refs/heads/master@{#392602}

TBR=nednguyen@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 610338 

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

[modify] https://crrev.com/527af54566e2435c62dc6a7acf3907e14f7c17af/tools/perf/benchmarks/smoothness.py
[modify] https://crrev.com/527af54566e2435c62dc6a7acf3907e14f7c17af/tools/perf/benchmarks/v8.py

Status: Fixed (was: Assigned)
Looks like smoothness/v8.key_mobile_sites_smooth benchmarks back on track!
https://uberchromegw.corp.google.com/i/chromium.perf/builders/Android%20Nexus5%20Perf%20%283%29/builds/1670

Sign in to add a comment