Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(125)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1924683003: [making a diff] Remove the old task manager view (Closed)

Created:
4 years, 7 months ago by tapted
Modified:
4 years, 7 months ago
Reviewers:
ncarter (slow)
CC:
chromium-reviews, davidben+watch_chromium.org, extensions-reviews_chromium.org, cbentzel+watch_chromium.org, skanuj+watch_chromium.org, melevin+watch_chromium.org, samarth+watch_chromium.org, tfarina, tburkard+watch_chromium.org, dhollowa+watch_chromium.org, dougw+watch_chromium.org, donnd+watch_chromium.org, gavinp+prer_chromium.org, jfweitz+watch_chromium.org, David Black, asvitkine+watch_chromium.org, kmadhusu+watch_chromium.org, chromium-apps-reviews_chromium.org, Jered
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[making a diff] Remove the old task manager view BUG=604806 patch from issue 1912773002 at patchset 120001 (http://crrev.com/1912773002#ps120001)

Patch Set 1 : baseline from crrev/1912773002 #

Patch Set 2 : Plus revert r389779 #

Patch Set 3 : Diff to get mac_views_browser=1 happy #

Patch Set 4 : switches mac_views_browser=1 to test we get past compile #

Patch Set 5 : we don't. add NotifyOldTaskManagerBytesRead #

Patch Set 6 : delete disabled tests #

Total comments: 1

Patch Set 7 : Rebase to master @r392280 #

Patch Set 8 : Rebase. EGads so many conflicts #

Patch Set 9 : Resolve rejects. chrome builds #

Patch Set 10 : rebase to master? #

Patch Set 11 : chrome compiles clean #

Patch Set 12 : rebase to https://codereview.chromium.org/1912773002/#ps220001 #

Patch Set 13 : just flips mac_views_browser =1 #

Patch Set 14 : Fix browser_tests #

Patch Set 15 : rebase to new master [conflicts] #

Patch Set 16 : fix nonmac #

Unified diffs Side-by-side diffs Delta from patch set Stats (+62 lines, -49 lines) Patch
M build/common.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/task_management/task_manager_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +1 line, -10 lines 0 comments Download
M chrome/browser/task_management/task_manager_tester.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +8 lines, -0 lines 0 comments Download
M chrome/browser/task_management/task_manager_tester.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +1 line, -10 lines 0 comments Download
D chrome/browser/task_manager/legacy_task_manager_tester.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +0 lines, -23 lines 0 comments Download
M chrome/browser/task_manager/legacy_task_manager_tester.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +20 lines, -5 lines 0 comments Download
A chrome/browser/task_manager/task_manager_tester_nonmac.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +30 lines, -0 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -0 lines 0 comments Download

Depends on Patchset:

Issue 2006513002 Patch 1

Messages

Total messages: 6 (2 generated)
ncarter (slow)
https://codereview.chromium.org/1924683003/diff/120001/chrome/browser/task_manager/task_manager_browsertest_util.h File chrome/browser/task_manager/task_manager_browsertest_util.h (right): https://codereview.chromium.org/1924683003/diff/120001/chrome/browser/task_manager/task_manager_browsertest_util.h#newcode21 chrome/browser/task_manager/task_manager_browsertest_util.h:21: #endif // defined(OS_MACOSX) FYI, This is the opposite of ...
4 years, 7 months ago (2016-04-27 16:50:53 UTC) #3
tapted
https://codereview.chromium.org/1924683003/diff/120001/chrome/browser/task_ma... File chrome/browser/task_manager/task_manager_browsertest_util.h (right): https://codereview.chromium.org/1924683003/diff/120001/chrome/browser/task_ma... chrome/browser/task_manager/task_manager_browsertest_util.h:21: #endif // defined(OS_MACOSX) On 2016/04/27 16:50:53, ncarter wrote: > ...
4 years, 7 months ago (2016-04-28 03:02:27 UTC) #4
ncarter (slow)
On 2016/04/28 03:02:27, tapted wrote: > https://codereview.chromium.org/1924683003/diff/120001/chrome/browser/task_ma... > File chrome/browser/task_manager/task_manager_browsertest_util.h (right): > > https://codereview.chromium.org/1924683003/diff/120001/chrome/browser/task_ma... > ...
4 years, 7 months ago (2016-05-11 17:18:56 UTC) #5
tapted
4 years, 7 months ago (2016-05-12 00:46:19 UTC) #6 
On 2016/05/11 17:18:56, ncarter wrote:
> On 2016/04/28 03:02:27, tapted wrote:
> >
>
https://codereview.chromium.org/1924683003/diff/120001/chrome/browser/task_ma...
> > File chrome/browser/task_manager/task_manager_browsertest_util.h (right):
> > 
> >
>
https://codereview.chromium.org/1924683003/diff/120001/chrome/browser/task_ma...
> > chrome/browser/task_manager/task_manager_browsertest_util.h:21: #endif  //
> > defined(OS_MACOSX)
> > On 2016/04/27 16:50:53, ncarter wrote:
> > > FYI, This is the opposite of what I'm planning on doing in
> > > https://codereview.chromium.org/1922683003/.
> > 
> > oooh nice - yeah I like the approach there - that should help resolve the
> > remaining link errors in patchset 6 here. I'll update this once
> > http://crrev.com/1912773002#ps120001 has been rebased on top of that. I
think
> > you'll still need the diffs here represented by patchset 5 vs patchset 2.
> 
> FYI, my tests changes have all landed. What is our plan is for proceeding with
> this CL, or ahmed's version?

https://codereview.chromium.org/1912773002 needs to be rebased. Then, it might
all be good. However, it will likely still need some tweaks so that the
`compile` step at go/macviewsbuilder doesn't regress -- that's what the CL here
is to help with. The diff of Patchset 5 against Patchset 2 is probably the
essence of what still needs to change for
https://codereview.chromium.org/191277300 to land cleanly.

Powered by Google App Engine
This is Rietveld 408576698