New issue
Advanced search Search tips

Issue 684042 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Task Manager background-refresh mask is broken

Project Member Reported by w...@chromium.org, Jan 23 2017

Issue description

I introduced a bug in this with a simple typo in https://codereview.chromium.org/2634573003

Will revert that CL and merge the revert, then re-land a fixed patch.
 

Comment 1 by w...@chromium.org, Jan 23 2017

Owner: w...@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 23 2017

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

commit 01f28a09b779a3081b4c485dae8e25e7fe8eba40
Author: wez <wez@chromium.org>
Date: Mon Jan 23 21:12:34 2017

Revert of Query NaCl processes' GDB debug port on the correct thread. (patchset #1 id:1 of https://codereview.chromium.org/2634573003/ )

Reason for revert:
This fix introduced a typo in the background refresh flags mask, breaking it.

Original issue's description:
> Query NaCl processes' GDB debug port on the correct thread.
>
> TaskGroup previously called NaClBrowser on the UI thread, to refresh the
> GDB debug port information for NaCl processes. NaClBrowser is designed
> to be accessed only from the Browser's IO thread; this patch fixes that.
>
> BUG= 630861 
>
> Review-Url: https://codereview.chromium.org/2634573003
> Cr-Commit-Position: refs/heads/master@{#444849}
> Committed: https://chromium.googlesource.com/chromium/src/+/f0bd838cce8870a95aec33be0b58151bd80f96aa

TBR=afakhry@chromium.org,bradnelson@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 630861 ,  684042 

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

[modify] https://crrev.com/01f28a09b779a3081b4c485dae8e25e7fe8eba40/chrome/browser/task_manager/sampling/task_group.cc
[modify] https://crrev.com/01f28a09b779a3081b4c485dae8e25e7fe8eba40/chrome/browser/task_manager/sampling/task_group.h

Comment 3 by w...@chromium.org, Jan 23 2017

Labels: Merge-Request-57
Requesting merge since this is a direct revert of a known-broken CL.
Project Member

Comment 4 by sheriffbot@chromium.org, Jan 24 2017

Labels: -Merge-Request-57 Hotlist-Merge-Review Merge-Review-57
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

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

Comment 5 by gov...@chromium.org, Jan 24 2017

Labels: -Merge-Review-57 Merge-Approved-57
Approving merge to M57 branch 2987 based on comment #3. Please merge ASAP. Thank you.
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 25 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/621ea63f81c4620ce4455c05cdbbffd4e0aa1af6

commit 621ea63f81c4620ce4455c05cdbbffd4e0aa1af6
Author: wez <wez@chromium.org>
Date: Wed Jan 25 18:10:02 2017

Revert of Query NaCl processes' GDB debug port on the correct thread. (patchset #1 id:1 of https://codereview.chromium.org/2634573003/ )

Reason for revert:
This fix introduced a typo in the background refresh flags mask, breaking it.

Original issue's description:
> Query NaCl processes' GDB debug port on the correct thread.
>
> TaskGroup previously called NaClBrowser on the UI thread, to refresh the
> GDB debug port information for NaCl processes. NaClBrowser is designed
> to be accessed only from the Browser's IO thread; this patch fixes that.
>
> BUG= 630861 
>
> Review-Url: https://codereview.chromium.org/2634573003
> Cr-Commit-Position: refs/heads/master@{#444849}
> Committed: https://chromium.googlesource.com/chromium/src/+/f0bd838cce8870a95aec33be0b58151bd80f96aa

NOTRY=true
NOPRESUBMIT=true
TBR=afakhry@chromium.org,bradnelson@chromium.org
BUG= 630861 ,  684042 

Review-Url: https://codereview.chromium.org/2649063003
Cr-Commit-Position: refs/heads/master@{#445485}
(cherry picked from commit 01f28a09b779a3081b4c485dae8e25e7fe8eba40)

Review-Url: https://codereview.chromium.org/2652083003
Cr-Commit-Position: refs/branch-heads/2987@{#89}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/621ea63f81c4620ce4455c05cdbbffd4e0aa1af6/chrome/browser/task_manager/sampling/task_group.cc
[modify] https://crrev.com/621ea63f81c4620ce4455c05cdbbffd4e0aa1af6/chrome/browser/task_manager/sampling/task_group.h

Comment 7 by w...@chromium.org, Jan 25 2017

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 17 2017

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

commit 91503efd94b63d9be85603e7d1d6edf7e837f4c9
Author: wez <wez@chromium.org>
Date: Fri Feb 17 02:26:43 2017

Query NaCl processes' GDB debug port on the correct thread.

TaskGroup previously called NaClBrowser on the UI thread, to refresh the
GDB debug port information for NaCl processes. NaClBrowser is designed
to be accessed only from the Browser's IO thread; this patch fixes that.Fix NaCl debug stub port fetching in Task Manager.

This is a re-land of crrev.com/2634573003 which had a couple of issues.

BUG= 630861 ,  684042 

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

[modify] https://crrev.com/91503efd94b63d9be85603e7d1d6edf7e837f4c9/chrome/browser/task_manager/sampling/task_group.cc
[modify] https://crrev.com/91503efd94b63d9be85603e7d1d6edf7e837f4c9/chrome/browser/task_manager/sampling/task_group.h
[add] https://crrev.com/91503efd94b63d9be85603e7d1d6edf7e837f4c9/chrome/browser/task_manager/sampling/task_group_unittest.cc
[modify] https://crrev.com/91503efd94b63d9be85603e7d1d6edf7e837f4c9/chrome/browser/ui/task_manager/task_manager_table_model.cc
[modify] https://crrev.com/91503efd94b63d9be85603e7d1d6edf7e837f4c9/chrome/test/BUILD.gn

Sign in to add a comment