[DocServer] Updated Top Sites Sample Extension.

[DocServer] Updated Top Sites Sample Extension.

BUG=299634
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=226483

[François Beaufort, 2013-10-01 14:49:59]

The current "Time filter for History" uses an experimental API named
chrome.experimental.history.getMostVisited which has been promoted to
chrome.topSites.

This CL moves this Sample to the examples/api/topsites folder and clean it up.

[not at google - send to devlin, 2013-10-01 15:04:36]

trivial comments, lgtm.

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/examples/api/topsites/basic/manifest.json
(right):

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/examples/api/topsites/basic/manifest.json:7:
"default_popup": "popup.html"
there was an icon for the old extension, could you include that?

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/examples/api/topsites/basic/popup.js (right):

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/examples/api/topsites/basic/popup.js:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
it would be nice if this file were detected as a move from the other one (there
is some "git cl upload" flag to set similarity). if not (i.e. if it doesn't make
sense), then update this copyright year.

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/examples/api/topsites/basic/popup.js:14: function
buildPopupDom(data) {
s/data/historyItems/

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/examples/api/topsites/basic/popup.js:29:
ol.appendChild(li);
i'm gonna nitpick here for no other reason than DOM API verbosity irritates me.
appendChild(...) returns the appended child, so you can write slightly more
concise code like:

for (...) {
  var li = ol.appendChild(document.createElement('li'));
  var a = li.appendChild(document.createElement('a'));
  ...
}

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/examples/api/topsites/basic/popup.js:35: });
cleanup: chrome.topSites.get(buildPopupDom);

[François Beaufort, 2013-10-02 12:44:59]

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/examples/api/topsites/basic/manifest.json
(right):

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/examples/api/topsites/basic/manifest.json:7:
"default_popup": "popup.html"
The old icon is actually not a valid file as you can see at
http://src.chromium.org/svn/trunk/src/chrome/common/extensions/docs/examples/...

On 2013/10/01 15:04:36, kalman wrote:
> there was an icon for the old extension, could you include that?

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/examples/api/topsites/basic/popup.js (right):

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/examples/api/topsites/basic/popup.js:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
I've played as much as possible with "git cl upload --similarity" and it never
worked as intended.
I really wish "git mv" would be considered but it's not ;(

So I ended up updating the Copyright year.


On 2013/10/01 15:04:36, kalman wrote:
> it would be nice if this file were detected as a move from the other one
(there
> is some "git cl upload" flag to set similarity). if not (i.e. if it doesn't
make
> sense), then update this copyright year.

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/examples/api/topsites/basic/popup.js:14: function
buildPopupDom(data) {
On 2013/10/01 15:04:36, kalman wrote:
> s/data/historyItems/

Done.

[not at google - send to devlin, 2013-10-02 14:48:12]

lgtm

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/examples/api/topsites/basic/manifest.json
(right):

https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/examples/api/topsites/basic/manifest.json:7:
"default_popup": "popup.html"
On 2013/10/02 12:44:59, François Beaufort wrote:
> The old icon is actually not a valid file as you can see at
>
http://src.chromium.org/svn/trunk/src/chrome/common/extensions/docs/examples/...
> 
> On 2013/10/01 15:04:36, kalman wrote:
> > there was an icon for the old extension, could you include that?
> 

Ah, oops... I would suggest finding an appropriate icon somewhere in the chrome
repo but it hardly seems worth it.

[commit-bot: I haz the power, 2013-10-02 16:59:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/beaufort.francois@gmail.com/25389003/2...

[François Beaufort, 2013-10-02 16:59:51]

On 2013/10/02 14:48:12, kalman wrote:
> lgtm
> 
>
https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
> File chrome/common/extensions/docs/examples/api/topsites/basic/manifest.json
> (right):
> 
>
https://codereview.chromium.org/25389003/diff/1/chrome/common/extensions/docs...
> chrome/common/extensions/docs/examples/api/topsites/basic/manifest.json:7:
> "default_popup": "popup.html"
> On 2013/10/02 12:44:59, François Beaufort wrote:
> > The old icon is actually not a valid file as you can see at
> >
>
http://src.chromium.org/svn/trunk/src/chrome/common/extensions/docs/examples/...
> > 
> > On 2013/10/01 15:04:36, kalman wrote:
> > > there was an icon for the old extension, could you include that?
> > 
> 
> Ah, oops... I would suggest finding an appropriate icon somewhere in the
chrome
> repo but it hardly seems worth it.

Added a browser action icon copied from another chrome extension and committed
the CQ.

[commit-bot: I haz the power, 2013-10-02 17:12:55]

Change committed as 226483