Eliminate sending NOTIFICATION_HISTORY_KEYWORD_SEARCH_TERM_DELETED

Eliminate sending NOTIFICATION_HISTORY_KEYWORD_SEARCH_TERM_DELETED

Added OnKeywordSearchTermDeleted inside HistoryServiceObserver.
Notification has been removed and its detail from chrome_notification_types.

BUG=373321
Committed: https://crrev.com/1fec74a41b8d0b02b4416b74f27f21dc024c3cce
Cr-Commit-Position: refs/heads/master@{#312788}

[Abhishek, 2015-01-15 16:05:12]

abhishek.a21@samsung.com changed reviewers:
+ newt@chromium.org, sdefresne@chromium.org, sky@chromium.org

[Abhishek, 2015-01-15 16:05:13]

PTAL !

[sdefresne, 2015-01-15 17:53:56]

https://codereview.chromium.org/830483005/diff/1/chrome/browser/android/provi...
File chrome/browser/android/provider/chrome_browser_provider.cc (left):

https://codereview.chromium.org/830483005/diff/1/chrome/browser/android/provi...
chrome/browser/android/provider/chrome_browser_provider.cc:1643: DCHECK_EQ(type,
chrome::NOTIFICATION_HISTORY_URLS_DELETED);
Do not remove those lines.

https://codereview.chromium.org/830483005/diff/1/chrome/browser/android/provi...
File chrome/browser/android/provider/chrome_browser_provider.cc (right):

https://codereview.chromium.org/830483005/diff/1/chrome/browser/android/provi...
chrome/browser/android/provider/chrome_browser_provider.cc:1642:
OnHistoryChanged();
Remove, ChromeBrowserProvider was not calling OnHistoryChanged when it received
NOTIFICATION_HISTORY_KEYWORD_SEARCH_TERM_DELETED, only when it was receiving
NOTIFICATION_HISTORY_URLS_DELETED.

[Abhishek, 2015-01-16 05:55:58]

updated chrome_browser_provider.
PTAL!

https://codereview.chromium.org/830483005/diff/1/chrome/browser/android/provi...
File chrome/browser/android/provider/chrome_browser_provider.cc (left):

https://codereview.chromium.org/830483005/diff/1/chrome/browser/android/provi...
chrome/browser/android/provider/chrome_browser_provider.cc:1643: DCHECK_EQ(type,
chrome::NOTIFICATION_HISTORY_URLS_DELETED);
On 2015/01/15 17:53:55, sdefresne wrote:
> Do not remove those lines.

Done.

https://codereview.chromium.org/830483005/diff/1/chrome/browser/android/provi...
File chrome/browser/android/provider/chrome_browser_provider.cc (right):

https://codereview.chromium.org/830483005/diff/1/chrome/browser/android/provi...
chrome/browser/android/provider/chrome_browser_provider.cc:1642:
OnHistoryChanged();
On 2015/01/15 17:53:55, sdefresne wrote:
> Remove, ChromeBrowserProvider was not calling OnHistoryChanged when it
received
> NOTIFICATION_HISTORY_KEYWORD_SEARCH_TERM_DELETED, only when it was receiving
> NOTIFICATION_HISTORY_URLS_DELETED.

Done.

[sdefresne, 2015-01-16 06:56:00]

LGTM once the comments have been updated.

https://codereview.chromium.org/830483005/diff/20001/chrome/browser/history/h...
File chrome/browser/history/history_service.h (right):

https://codereview.chromium.org/830483005/diff/20001/chrome/browser/history/h...
chrome/browser/history/history_service.h:627: // deleted. |url_id| id of the url
row.
Style: comment must be correct english sentences

|url_id| is the id of the URL row.

https://codereview.chromium.org/830483005/diff/20001/components/history/core/...
File components/history/core/browser/history_service_observer.h (right):

https://codereview.chromium.org/830483005/diff/20001/components/history/core/...
components/history/core/browser/history_service_observer.h:63: // |url_id| id of
the url row.
Style: comment must be correct english sentences

// Sent when a keyword search term is deleted.
// |url_id| is the id of the URL row.

[Abhishek, 2015-01-16 07:43:48]

Updated as per the comment.
@newt:
PTAL!

https://codereview.chromium.org/830483005/diff/20001/chrome/browser/history/h...
File chrome/browser/history/history_service.h (right):

https://codereview.chromium.org/830483005/diff/20001/chrome/browser/history/h...
chrome/browser/history/history_service.h:627: // deleted. |url_id| id of the url
row.
Thanks for the correction!
I was thinking it, like:
|url_id| is considered as heading and remaining is msg for |[ITEM]|.

https://codereview.chromium.org/830483005/diff/20001/components/history/core/...
File components/history/core/browser/history_service_observer.h (right):

https://codereview.chromium.org/830483005/diff/20001/components/history/core/...
components/history/core/browser/history_service_observer.h:63: // |url_id| id of
the url row.
On 2015/01/16 06:56:00, sdefresne wrote:
> Style: comment must be correct english sentences
> 
> // Sent when a keyword search term is deleted.
> // |url_id| is the id of the URL row.

Done.

[newt (away), 2015-01-16 18:58:03]

android files lgtm

https://codereview.chromium.org/830483005/diff/40001/chrome/browser/android/p...
File chrome/browser/android/provider/chrome_browser_provider.h (right):

https://codereview.chromium.org/830483005/diff/40001/chrome/browser/android/p...
chrome/browser/android/provider/chrome_browser_provider.h:199: history::URLID
url_id) override{};
space before {

[sdefresne, 2015-01-17 18:23:23]

https://codereview.chromium.org/830483005/diff/40001/chrome/browser/android/p...
File chrome/browser/android/provider/chrome_browser_provider.h (right):

https://codereview.chromium.org/830483005/diff/40001/chrome/browser/android/p...
chrome/browser/android/provider/chrome_browser_provider.h:199: history::URLID
url_id) override{};
On 2015/01/16 at 18:58:03, newt wrote:
> space before {

Nit: no ; after the {} too, and could you not inline this method but put it in
the implementation file?

[Abhishek, 2015-01-19 06:13:01]

Updated chrome/browser/android files as commented.
@sky:
PTAL!

https://codereview.chromium.org/830483005/diff/40001/chrome/browser/android/p...
File chrome/browser/android/provider/chrome_browser_provider.h (right):

https://codereview.chromium.org/830483005/diff/40001/chrome/browser/android/p...
chrome/browser/android/provider/chrome_browser_provider.h:199: history::URLID
url_id) override{};
On 2015/01/16 18:58:03, newt wrote:
> space before {

Done.

https://codereview.chromium.org/830483005/diff/40001/chrome/browser/android/p...
chrome/browser/android/provider/chrome_browser_provider.h:199: history::URLID
url_id) override{};
On 2015/01/17 18:23:23, sdefresne wrote:
> On 2015/01/16 at 18:58:03, newt wrote:
> > space before {
> 
> Nit: no ; after the {} too, and could you not inline this method but put it in
> the implementation file?

Done.

[sky, 2015-01-22 17:08:55]

chrome/browser/chrome_notification_types.h LGTM

[Abhishek, 2015-01-23 05:18:50]

The CQ bit was checked by abhishek.a21@samsung.com

[commit-bot: I haz the power, 2015-01-23 05:19:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/830483005/60001

[commit-bot: I haz the power, 2015-01-23 06:45:57]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-01-23 06:46:45]

Patchset 4 (id:??) landed as
https://crrev.com/1fec74a41b8d0b02b4416b74f27f21dc024c3cce
Cr-Commit-Position: refs/heads/master@{#312788}