[Contextual Search] Fix history removal regression.

[Contextual Search] Fix history removal regression.

BUG=545617
BUG=545614
Committed: https://crrev.com/9978acd0c0d83d104886f035f9a85327e67722ba
Cr-Commit-Position: refs/heads/master@{#355178}

[pedro (no code reviews), 2015-10-20 19:12:28]

Description was changed from

==========
[Contextual Search] Fix history removal regression.

BUG=545617
BUG=545614
==========

to

==========
[Contextual Search] Fix history removal regression.

BUG=545617
BUG=545614
==========

[pedro (no code reviews), 2015-10-20 19:12:28]

pedrosimonetti@chromium.org changed reviewers:
+ aurimas@chromium.org, donnd@chromium.org

[pedro (no code reviews), 2015-10-20 19:13:18]

Hey guys, please take a look at this change. It fixes a recently introduced
regression I want to merge this into M47, and it also fixes an old problem with
our history removal logic.

I will add tests in a followup CL just for M48.

[Donn Denman, 2015-10-20 20:34:07]

lgtm

[aurimas (slooooooooow), 2015-10-20 20:51:04]

lgtm

https://chromiumcodereview.appspot.com/1412423003/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java
(right):

https://chromiumcodereview.appspot.com/1412423003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java:239:
// Must be called before destroying Content.
Can you expand the comment explaining why it needs to be called before?

https://chromiumcodereview.appspot.com/1412423003/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
(right):

https://chromiumcodereview.appspot.com/1412423003/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:918:
// TODO(pedrosimonetti): Consider having this feature builtin into the Content.
What do you mean by "the Content"? ContentViewCore? Is there a bug for this
TODO?

[pedro (no code reviews), 2015-10-20 22:18:11]

https://codereview.chromium.org/1412423003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java
(right):

https://codereview.chromium.org/1412423003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java:239:
// Must be called before destroying Content.
On 2015/10/20 20:51:04, aurimas wrote:
> Can you expand the comment explaining why it needs to be called before?

Done.

https://codereview.chromium.org/1412423003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
(right):

https://codereview.chromium.org/1412423003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:918:
// TODO(pedrosimonetti): Consider having this feature builtin into the Content.
On 2015/10/20 20:51:04, aurimas wrote:
> What do you mean by "the Content"? ContentViewCore? Is there a bug for this
> TODO?

By "Content" I mean "OverlayPanelContent". Filed a bug for that:
crbug.com/545693

[pedro (no code reviews), 2015-10-20 22:18:21]

The CQ bit was checked by pedrosimonetti@chromium.org

[pedro (no code reviews), 2015-10-20 22:18:22]

The patchset sent to the CQ was uploaded after l-g-t-m from donnd@chromium.org,
aurimas@chromium.org
Link to the patchset: https://codereview.chromium.org/1412423003/#ps20001
(title: "addressing comments")

[commit-bot: I haz the power, 2015-10-20 22:19:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412423003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412423003/20001

[commit-bot: I haz the power, 2015-10-20 23:03:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-20 23:04:37]

Patchset 2 (id:??) landed as
https://crrev.com/9978acd0c0d83d104886f035f9a85327e67722ba
Cr-Commit-Position: refs/heads/master@{#355178}