Fix bug avi found.

Fix bug avi found.

BUG=

[Avi (use Gerrit), 2016-08-01 18:56:02]

avi@chromium.org changed reviewers:
+ avi@chromium.org

[Avi (use Gerrit), 2016-08-01 18:56:02]

Folding this into https://codereview.chromium.org/2195873002

[afakhry, 2016-08-01 20:53:53]

afakhry@chromium.org changed reviewers:
+ afakhry@chromium.org

[afakhry, 2016-08-01 20:53:54]

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
File chrome/browser/ui/task_manager/task_manager_table_model.cc (right):

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
chrome/browser/ui/task_manager/task_manager_table_model.cc:565: base::ProcessId
process_id =
const

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
chrome/browser/ui/task_manager/task_manager_table_model.cc:571: i--;
++i?

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
chrome/browser/ui/task_manager/task_manager_table_model.cc:575: limit++;
++limit?

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
chrome/browser/ui/task_manager/task_manager_table_model.cc:578: *out_length =
limit - i;
You can ignore the following: :D

Hmmm, that's a very robust and cool fix! But remember Sean Parent? ;)

What you're really trying to do is search forward and backward for the first
task not on the same proc, right?

  const base::ProcessId process_id =
      observed_task_manager()->GetProcessId(tasks_[row_index]);
  auto not_same_proc_pred = [this, process_id](TaskId id) {
    return process_id != observed_task_manager()->GetProcessId(id);
  };

  using iter_t = std::vector<TaskId>::iterator;
  iter_t row_iter = tasks_.begin() + row_index;
  *out_start =
      tasks_.rend() - std::find_if(std::reverse_iterator<iter_t>(row_iter),
                                   tasks_.rend(),
                                   not_same_proc_pred);
  auto end = std::find_if(++row_iter, tasks_.end(), not_same_proc_pred);
  *out_length = end - (tasks_.begin() + *out_start);

[Avi (use Gerrit), 2016-08-01 20:57:39]

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
File chrome/browser/ui/task_manager/task_manager_table_model.cc (right):

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
chrome/browser/ui/task_manager/task_manager_table_model.cc:571: i--;
On 2016/08/01 20:53:53, afakhry wrote:
> ++i?

--i?

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
chrome/browser/ui/task_manager/task_manager_table_model.cc:578: *out_length =
limit - i;
On 2016/08/01 20:53:53, afakhry wrote:
> You can ignore the following: :D
> 
> Hmmm, that's a very robust and cool fix! But remember Sean Parent? ;)
> 
> What you're really trying to do is search forward and backward for the first
> task not on the same proc, right?
> 
>   const base::ProcessId process_id =
>       observed_task_manager()->GetProcessId(tasks_[row_index]);
>   auto not_same_proc_pred = [this, process_id](TaskId id) {
>     return process_id != observed_task_manager()->GetProcessId(id);
>   };
> 
>   using iter_t = std::vector<TaskId>::iterator;
>   iter_t row_iter = tasks_.begin() + row_index;
>   *out_start =
>       tasks_.rend() - std::find_if(std::reverse_iterator<iter_t>(row_iter),
>                                    tasks_.rend(),
>                                    not_same_proc_pred);
>   auto end = std::find_if(++row_iter, tasks_.end(), not_same_proc_pred);
>   *out_length = end - (tasks_.begin() + *out_start);

Yes, I actually thought of Sean's talk. I searched in <algorithm> for this kind
of thing, but the closest thing was equal_range and that required a sorted span.
By the time you set up two different find_ifs like you did, it didn't feel like
it was worth the hassle.

[afakhry, 2016-08-01 21:02:59]

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
File chrome/browser/ui/task_manager/task_manager_table_model.cc (right):

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
chrome/browser/ui/task_manager/task_manager_table_model.cc:571: i--;
On 2016/08/01 20:57:39, Avi wrote:
> On 2016/08/01 20:53:53, afakhry wrote:
> > ++i?
> 
> --i?

Oops, yes, sorry! :)

https://codereview.chromium.org/2202563003/diff/1/chrome/browser/ui/task_mana...
chrome/browser/ui/task_manager/task_manager_table_model.cc:578: *out_length =
limit - i;
On 2016/08/01 20:57:39, Avi wrote:
> On 2016/08/01 20:53:53, afakhry wrote:
> > You can ignore the following: :D
> > 
> > Hmmm, that's a very robust and cool fix! But remember Sean Parent? ;)
> > 
> > What you're really trying to do is search forward and backward for the first
> > task not on the same proc, right?
> > 
> >   const base::ProcessId process_id =
> >       observed_task_manager()->GetProcessId(tasks_[row_index]);
> >   auto not_same_proc_pred = [this, process_id](TaskId id) {
> >     return process_id != observed_task_manager()->GetProcessId(id);
> >   };
> > 
> >   using iter_t = std::vector<TaskId>::iterator;
> >   iter_t row_iter = tasks_.begin() + row_index;
> >   *out_start =
> >       tasks_.rend() - std::find_if(std::reverse_iterator<iter_t>(row_iter),
> >                                    tasks_.rend(),
> >                                    not_same_proc_pred);
> >   auto end = std::find_if(++row_iter, tasks_.end(), not_same_proc_pred);
> >   *out_length = end - (tasks_.begin() + *out_start);
> 
> Yes, I actually thought of Sean's talk. I searched in <algorithm> for this
kind
> of thing, but the closest thing was equal_range and that required a sorted
span.
> By the time you set up two different find_ifs like you did, it didn't feel
like
> it was worth the hassle.

It's certainly not worth the hassle, that's why I said you can ignore :)