More info:

Call 2 has the stack trace:

task_management::TaskManagerMac::OnItemsRemoved(int, int)
task_management::TaskManagerTableModel::OnTaskToBeRemoved(long long)
task_management::TaskManagerInterface::NotifyObserversOnTaskToBeRemoved(long long)
task_management::TaskManagerImpl::TaskRemoved(task_management::Task*)
task_management::TaskProvider::NotifyObserverTaskRemoved(task_management::Task*) const
task_management::WebContentsEntry::ClearAllTasks(bool)
task_management::WebContentsEntry::RenderViewReady()
content::WebContentsImpl::RenderViewReady(content::RenderViewHost*)

task_management::WebContentsEntry::RenderViewReady is:

  void RenderViewReady() override {
    ClearAllResources(true);
    CreateAllResources();
  }

Apparently that's not hooked up to grouping. Should it?

It looks like it needs to be.

I added logging to the grouping code, and we get:

>> OnItemsAdded(7, 1)
] extending the selection of index 6 to start at 6 and be of length 2
] extending the selection of index 7 to start at 6 and be of length 2
>> OnItemsRemoved(7, 1)
] extending the selection of index 6 to start at 6 and be of length 2
] Yikes! The item at index 7 belongs to a group starting at 7 and of length 3! Ignoring.
>> OnItemsAdded(7, 1)
] extending the selection of index 6 to start at 6 and be of length 2
] extending the selection of index 7 to start at 6 and be of length 2

At that moment, the TaskManagerTableModel is serving data inconsistent with itself.

The mismatch seems to be here:

void TaskManagerTableModel::OnTaskToBeRemoved(TaskId id) {
  auto index = std::find(tasks_.begin(), tasks_.end(), id);
  if (index == tasks_.end())
    return;
  auto removed_index = index - tasks_.begin();
  tasks_.erase(index);
  if (table_model_observer_)
    table_model_observer_->OnItemsRemoved(removed_index, 1);
}

The notification is that an item *will* be removed. The table model then removes it from its data, and tells the UI that the item *was* removed. The UI then asks the table model for grouping information, but TaskManagerTableModel::GetRowsGroupRange calls back to the task manager for its data, but the task manager is in the "pre-removal" state.

If the table model is going to transform "will be" removed to "was" removed, it has to translate the grouping as well.

Awesome find. Thanks for digging into this.

If I understand correctly, there are are at least two bugs here:

 - The behavior of GetRowsGroupRange is inconsistent in the deletion case, since we've removed the item from TaskManagerTableModel::|tasks_| but the deleted item is still first in its group according to the TaskManager. I bet we can create a unittest for this case.

 - The RenderViewReady logic causes an unnecessary Remove/Add that exacerbates the first bug.

We should also inspect the code to see if there are other cases like GetRowsGroupRange.

Great find, Avi! Thanks!

Does switching the order fixes the issue? i.e.:

  if (table_model_observer_)
    table_model_observer_->OnItemsRemoved(removed_index, 1);
  tasks_.erase(index);

The wrong number of tasks per process in this task-removal transient state is probably what we want to avoid: https://cs.chromium.org/chromium/src/chrome/browser/ui/task_manager/task_manager_table_model.cc?q=TaskManagerTableModel::GetRowsGroupRange&sq=package:chromium&dr=C&l=574, correct?

https://codereview.chromium.org/2202563003/ is what I was thinking.

Re #7, that doesn't make much sense. The callback is "items were removed" so not actually removing it makes it worse.

I'm trying out Nick's patch to see if it works.

Nick, Ahmed, can you look at https://codereview.chromium.org/2195873002/?

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

commit 1c6f3ff833b80b5041d8756a27a465e1bacb7ffa
Author: avi <avi@chromium.org>
Date: Mon Aug 01 22:33:35 2016

Fix task groups to always be self-consistent.

BUG= 632775 
TEST=TaskManagerBrowserTest.*

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

[modify] https://crrev.com/1c6f3ff833b80b5041d8756a27a465e1bacb7ffa/chrome/browser/task_management/task_manager_browsertest.cc
[modify] https://crrev.com/1c6f3ff833b80b5041d8756a27a465e1bacb7ffa/chrome/browser/task_management/task_manager_tester.cc
[modify] https://crrev.com/1c6f3ff833b80b5041d8756a27a465e1bacb7ffa/chrome/browser/task_management/task_manager_tester.h
[modify] https://crrev.com/1c6f3ff833b80b5041d8756a27a465e1bacb7ffa/chrome/browser/task_manager/legacy_task_manager_tester.cc
[modify] https://crrev.com/1c6f3ff833b80b5041d8756a27a465e1bacb7ffa/chrome/browser/ui/task_manager/task_manager_table_model.cc