Verify that previous_selected_line is within bounds before passing to

Verify that previous_selected_line is within bounds before passing to
view.

BUG=704456
Review-Url: https://codereview.chromium.org/2766263007
Cr-Commit-Position: refs/heads/master@{#459826}
Committed: https://chromium.googlesource.com/chromium/src/+/337728d63a7a159a6a7be5e7255246e9d42509a2

[csashi, 2017-03-23 21:56:36]

csashi@google.com changed reviewers:
+ estade@chromium.org

[csashi, 2017-03-23 21:56:37]

Hi Evan,
I will add a unit-test for this case but wanted to send the change first for
your review. Thanks,
-sashi.

[Evan Stade, 2017-03-23 23:06:49]

https://codereview.chromium.org/2766263007/diff/1/chrome/browser/ui/autofill/...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.cc (right):

https://codereview.chromium.org/2766263007/diff/1/chrome/browser/ui/autofill/...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:362:
SetSelectedLine(base::nullopt);
this is the problem, right? Can we change this line to selected_line_.reset()?

we're doing a lot of excess work (repaints, ClearPreviewedForm x 2) by calling
SetSelectedLine.

[csashi, 2017-03-23 23:45:25]

Hi Evan,
Please take a look. Thanks,
-sashi.

https://codereview.chromium.org/2766263007/diff/1/chrome/browser/ui/autofill/...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.cc (right):

https://codereview.chromium.org/2766263007/diff/1/chrome/browser/ui/autofill/...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:362:
SetSelectedLine(base::nullopt);
On 2017/03/23 23:06:49, Evan Stade wrote:
> this is the problem, right? Can we change this line to selected_line_.reset()?
> 
> we're doing a lot of excess work (repaints, ClearPreviewedForm x 2) by calling
> SetSelectedLine.

Done. Thanks for the suggestion. To the best of my verification, the effect is
the same minus the bug and is simpler.

[csashi, 2017-03-26 16:32:12]

The CQ bit was checked by csashi@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-26 16:32:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-26 18:16:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-26 18:16:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2017-03-27 17:21:17]

lgtm

https://codereview.chromium.org/2766263007/diff/20001/chrome/browser/ui/autof...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.cc (right):

https://codereview.chromium.org/2766263007/diff/20001/chrome/browser/ui/autof...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:403: 
nit: no need for this extra newline

[csashi, 2017-03-27 17:25:22]

https://codereview.chromium.org/2766263007/diff/20001/chrome/browser/ui/autof...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.cc (right):

https://codereview.chromium.org/2766263007/diff/20001/chrome/browser/ui/autof...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:403: 
On 2017/03/27 17:21:16, Evan Stade wrote:
> nit: no need for this extra newline

Done.

[csashi, 2017-03-27 17:25:30]

The CQ bit was checked by csashi@google.com

[csashi, 2017-03-27 17:25:31]

The patchset sent to the CQ was uploaded after l-g-t-m from estade@chromium.org
Link to the patchset: https://codereview.chromium.org/2766263007/#ps40001
(title: "Removes extra newline.")

[commit-bot: I haz the power, 2017-03-27 17:25:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-27 18:09:43]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490635530962710,
"parent_rev": "2fc3ba8587526eb5e5f7455ca463b0c166b1ee95", "commit_rev":
"337728d63a7a159a6a7be5e7255246e9d42509a2"}

[commit-bot: I haz the power, 2017-03-27 18:10:34]

Description was changed from

==========
Verify that previous_selected_line is within bounds before passing to
view.

BUG=704456
==========

to

==========
Verify that previous_selected_line is within bounds before passing to
view.

BUG=704456

Review-Url: https://codereview.chromium.org/2766263007
Cr-Commit-Position: refs/heads/master@{#459826}
Committed:
https://chromium.googlesource.com/chromium/src/+/337728d63a7a159a6a7be5e72552...
==========

[commit-bot: I haz the power, 2017-03-27 18:10:35]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/337728d63a7a159a6a7be5e72552...