Added page score to the ntp suggestions tab.

Added page score to the ntp suggestions tab.


BUG=
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=134454

[GeorgeY, 2012-04-19 01:16:24]

LGTM with some comments from me - you will need owners approval as well

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/history...
File chrome/browser/history/history.h (right):

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/history...
chrome/browser/history/history.h:374: QueryFilteredURLsCallback;
As long as we adding a new one - could we make it const ref to not copy
(potentially) a huge list?

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/history...
File chrome/browser/history/history_backend.cc (right):

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/history...
chrome/browser/history/history_backend.cc:113: FilteredURL MakeFilteredURL(const
PageUsageData& page_data) {
May be add an explicit constructor 
explicit FilteredURL(const PageUsageData& page_data)
instead? What do you think?

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/ui/webu...
File chrome/browser/ui/webui/ntp/suggestions_page_handler.h (right):

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/ui/webu...
chrome/browser/ui/webui/ntp/suggestions_page_handler.h:77:
history::FilteredURLList data);
Again, as long as we changing it, "const history::FilteredURLList& data" is much
better :)

[Rune Fevang, 2012-04-25 00:52:45]

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/history...
File chrome/browser/history/history.h (right):

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/history...
chrome/browser/history/history.h:374: QueryFilteredURLsCallback;
On 2012/04/19 01:16:24, GeorgeY wrote:
> As long as we adding a new one - could we make it const ref to not copy
> (potentially) a huge list?

Done.

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/history...
File chrome/browser/history/history_backend.cc (right):

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/history...
chrome/browser/history/history_backend.cc:113: FilteredURL MakeFilteredURL(const
PageUsageData& page_data) {
On 2012/04/19 01:16:24, GeorgeY wrote:
> May be add an explicit constructor 
> explicit FilteredURL(const PageUsageData& page_data)
> instead? What do you think?

Done.

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/ui/webu...
File chrome/browser/ui/webui/ntp/suggestions_page_handler.h (right):

https://chromiumcodereview.appspot.com/10115001/diff/1/chrome/browser/ui/webu...
chrome/browser/ui/webui/ntp/suggestions_page_handler.h:77:
history::FilteredURLList data);
On 2012/04/19 01:16:24, GeorgeY wrote:
> Again, as long as we changing it, "const history::FilteredURLList& data" is
much
> better :)

Done.

[Rune Fevang, 2012-04-25 01:01:36]

Added Brett and Evan for owners review.

[Evan Stade, 2012-04-27 02:32:06]

webui bit lgtm

http://codereview.chromium.org/10115001/diff/6001/chrome/browser/resources/nt...
File chrome/browser/resources/ntp4/suggestions_page.css (right):

http://codereview.chromium.org/10115001/diff/6001/chrome/browser/resources/nt...
chrome/browser/resources/ntp4/suggestions_page.css:62: /* Matches height of
title.  */
this comment needs updating

[Rune Fevang, 2012-04-27 19:34:29]

https://chromiumcodereview.appspot.com/10115001/diff/6001/chrome/browser/reso...
File chrome/browser/resources/ntp4/suggestions_page.css (right):

https://chromiumcodereview.appspot.com/10115001/diff/6001/chrome/browser/reso...
chrome/browser/resources/ntp4/suggestions_page.css:62: /* Matches height of
title.  */
On 2012/04/27 02:32:06, Evan Stade wrote:
> this comment needs updating

Done.

[brettw, 2012-04-27 21:09:21]

LGTM

https://chromiumcodereview.appspot.com/10115001/diff/16002/chrome/browser/his...
File chrome/browser/history/history_types.cc (right):

https://chromiumcodereview.appspot.com/10115001/diff/16002/chrome/browser/his...
chrome/browser/history/history_types.cc:339: {
Put the { on the previous line.

[Rune Fevang, 2012-04-27 22:06:25]

Thanks! Could one of you submit the patch for me? (George left the company.)

https://chromiumcodereview.appspot.com/10115001/diff/16002/chrome/browser/his...
File chrome/browser/history/history_types.cc (right):

https://chromiumcodereview.appspot.com/10115001/diff/16002/chrome/browser/his...
chrome/browser/history/history_types.cc:339: {
On 2012/04/27 21:09:21, brettw wrote:
> Put the { on the previous line.

Done.

[Evan Stade, 2012-04-28 00:35:49]

you should be able to click the commit box

[commit-bot: I haz the power, 2012-04-28 06:41:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rfevang@chromium.org/10115001/8008

[commit-bot: I haz the power, 2012-04-28 08:06:05]

Change committed as 134454