NTP Cleanup: Remove support for "icons" layout

NTP Cleanup: Remove support for "icons" layout

Previous CLs:
https://codereview.chromium.org/2677023003 removed the switches
https://codereview.chromium.org/2670123003 removed support on local NTP

BUG=601860, 687972
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2681723002
Cr-Commit-Position: refs/heads/master@{#449259}
Committed: https://chromium.googlesource.com/chromium/src/+/1d689c69e5bcd24c86ad43825bc2ea4410f12acf

[Marc Treib, 2017-02-07 12:36:14]

Description was changed from

==========
NTP Cleanup: Remove support for "icons" layout

BUG=601860,687972
==========

to

==========
NTP Cleanup: Remove support for "icons" layout

BUG=601860,687972
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Marc Treib, 2017-02-07 12:38:29]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 12:38:43]

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

[Marc Treib, 2017-02-07 12:39:17]

treib@chromium.org changed reviewers:
+ mastiz@chromium.org

[Marc Treib, 2017-02-07 12:39:18]

If we're gonna do this, let's do it fully :)  PTAL!

[commit-bot: I haz the power, 2017-02-07 14:33:18]

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

[commit-bot: I haz the power, 2017-02-07 14:33:19]

Dry run: This issue passed the CQ dry run.

[mastiz, 2017-02-08 13:59:22]

LGTM!

Can you please elaborate the CL description with references to the preceding
related CL(s)?

https://codereview.chromium.org/2681723002/diff/1/chrome/renderer/resources/e...
File chrome/renderer/resources/extensions/searchbox_api.js (right):

https://codereview.chromium.org/2681723002/diff/1/chrome/renderer/resources/e...
chrome/renderer/resources/extensions/searchbox_api.js:116:
this.getMostVisitedItemData = function(restrictedId) {
Seems like the signature matches. Do you even need the lambda?

[Marc Treib, 2017-02-08 14:02:24]

Description was changed from

==========
NTP Cleanup: Remove support for "icons" layout

BUG=601860,687972
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
NTP Cleanup: Remove support for "icons" layout

Previous CL which removed the switches for enabling it:
https://codereview.chromium.org/2677023003

BUG=601860,687972
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Marc Treib, 2017-02-08 14:02:48]

Description updated - I'm only aware of the one preceding CL?

https://codereview.chromium.org/2681723002/diff/1/chrome/renderer/resources/e...
File chrome/renderer/resources/extensions/searchbox_api.js (right):

https://codereview.chromium.org/2681723002/diff/1/chrome/renderer/resources/e...
chrome/renderer/resources/extensions/searchbox_api.js:116:
this.getMostVisitedItemData = function(restrictedId) {
On 2017/02/08 13:59:22, mastiz wrote:
> Seems like the signature matches. Do you even need the lambda?

The inner GetMostVisitedItemData is a native function (see definition above), I
think it's not possible to expose that directly, hence the wrapper. All the
other exposed functions do the same thing.

[mastiz, 2017-02-09 08:20:06]

Still LGTM.

On 2017/02/08 14:02:48, Marc Treib wrote:
> Description updated - I'm only aware of the one preceding CL?

How about https://codereview.chromium.org/2670123003? (can it be a coincidence
that they share the last 5 digits?)

> 
>
https://codereview.chromium.org/2681723002/diff/1/chrome/renderer/resources/e...
> File chrome/renderer/resources/extensions/searchbox_api.js (right):
> 
>
https://codereview.chromium.org/2681723002/diff/1/chrome/renderer/resources/e...
> chrome/renderer/resources/extensions/searchbox_api.js:116:
> this.getMostVisitedItemData = function(restrictedId) {
> On 2017/02/08 13:59:22, mastiz wrote:
> > Seems like the signature matches. Do you even need the lambda?
> 
> The inner GetMostVisitedItemData is a native function (see definition above),
I
> think it's not possible to expose that directly, hence the wrapper. All the
> other exposed functions do the same thing.

Ack.

[Marc Treib, 2017-02-09 09:41:17]

Description was changed from

==========
NTP Cleanup: Remove support for "icons" layout

Previous CL which removed the switches for enabling it:
https://codereview.chromium.org/2677023003

BUG=601860,687972
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
NTP Cleanup: Remove support for "icons" layout

Previous CLs:
https://codereview.chromium.org/2677023003 removed the switches
https://codereview.chromium.org/2670123003 removed support on local NTP

BUG=601860,687972
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Marc Treib, 2017-02-09 09:41:42]

On 2017/02/09 08:20:06, mastiz wrote:
> Still LGTM.
> 
> On 2017/02/08 14:02:48, Marc Treib wrote:
> > Description updated - I'm only aware of the one preceding CL?
> 
> How about https://codereview.chromium.org/2670123003 (can it be a coincidence
> that they share the last 5 digits?)

D'oh, forgot my own CL :D   Added!

[Marc Treib, 2017-02-09 09:42:01]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2017-02-09 09:42:14]

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-02-09 10:43:35]

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

[commit-bot: I haz the power, 2017-02-09 10:43:36]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Marc Treib, 2017-02-09 10:48:44]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-09 10:49:03]

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-02-09 11:17:10]

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

[commit-bot: I haz the power, 2017-02-09 11:17:11]

Dry run: This issue passed the CQ dry run.

[Marc Treib, 2017-02-09 11:24:31]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2017-02-09 11:24:42]

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-02-09 11:28:14]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1486639471605880, "parent_rev":
"0ea3c20f46d0ce2139eaf03bba6a3dd79429c786", "commit_rev":
"1d689c69e5bcd24c86ad43825bc2ea4410f12acf"}

[commit-bot: I haz the power, 2017-02-09 11:28:50]

Description was changed from

==========
NTP Cleanup: Remove support for "icons" layout

Previous CLs:
https://codereview.chromium.org/2677023003 removed the switches
https://codereview.chromium.org/2670123003 removed support on local NTP

BUG=601860,687972
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
NTP Cleanup: Remove support for "icons" layout

Previous CLs:
https://codereview.chromium.org/2677023003 removed the switches
https://codereview.chromium.org/2670123003 removed support on local NTP

BUG=601860,687972
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2681723002
Cr-Commit-Position: refs/heads/master@{#449259}
Committed:
https://chromium.googlesource.com/chromium/src/+/1d689c69e5bcd24c86ad43825bc2...
==========

[commit-bot: I haz the power, 2017-02-09 11:28:54]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/1d689c69e5bcd24c86ad43825bc2...