Start axing NTP4 dead code.

Start axing NTP4 dead code.

"Recently closed" is always hidden and is first on the chopping block.

R=estade@chromium.org
BUG=329637
Committed: https://crrev.com/7894b5557c317e6302d3dde13b28dec8c3798d46
Cr-Commit-Position: refs/heads/master@{#329937}

[Dan Beam, 2015-05-13 23:18:01]

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/ntp_resource_cache.cc (left):

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:432:
l10n_util::GetStringUTF16(IDS_NEW_TAB_RECENTLY_CLOSED));
these are all still used from the flapjacks menu

[Dan Beam, 2015-05-13 23:48:48]

Patchset #2 (id:20001) has been deleted

[Evan Stade, 2015-05-14 00:09:45]

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/new_tab_page_handler.cc (left):

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/new_tab_page_handler.cc:165:
"ExtendedNewTabPage.TimeToClickRecentlyClosed", delta);
ah! this is one way to verify the code isn't used any more. Check uma to see if
there are any users hitting this bucket. Have you done so?

also, update histograms.xml?

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/ntp_resource_cache.cc (left):

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:432:
l10n_util::GetStringUTF16(IDS_NEW_TAB_RECENTLY_CLOSED));
On 2015/05/13 23:18:01, Dan Beam wrote:
> these are all still used from the flapjacks menu

should probably consider renaming them/reorganizing the string file

[Dan Beam, 2015-05-14 01:09:35]

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/new_tab_page_handler.cc (left):

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/new_tab_page_handler.cc:165:
"ExtendedNewTabPage.TimeToClickRecentlyClosed", delta);
On 2015/05/14 00:09:44, Evan Stade wrote:
> ah! this is one way to verify the code isn't used any more. Check uma to see
if
> there are any users hitting this bucket. Have you done so?
> 
> also, update histograms.xml?

can't find it in UMA nor is it in histograms.xml

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/ntp_resource_cache.cc (left):

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:432:
l10n_util::GetStringUTF16(IDS_NEW_TAB_RECENTLY_CLOSED));
On 2015/05/14 00:09:45, Evan Stade wrote:
> On 2015/05/13 23:18:01, Dan Beam wrote:
> > these are all still used from the flapjacks menu
> 
> should probably consider renaming them/reorganizing the string file

they're all still valid ("1 tab", "## tabs" and "Recently closed" are all still
used and validly named)

[jungshik at Google, 2015-05-14 08:02:03]

jshin@chromium.org changed reviewers:
+ jshin@chromium.org

[jungshik at Google, 2015-05-14 08:02:04]

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/ntp_resource_cache.cc (left):

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:440:
l10n_util::GetStringUTF16(IDS_NEW_TAB_RECENTLY_CLOSED_WINDOW_MULTIPLE));
IDS_NEW_TAB_RECENTLY_CLOSED_WINDOW_{SINGLE,MULTIPLE} have to be removed from
generated_resources.grd. My CL added IDS_NEW_TAB_RECENTLY_CLOSED_WINDOW (using
ICU plural syntax) but left the two above alone because this file still uses
them.

[Evan Stade, 2015-05-14 19:04:55]

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/new_tab_page_handler.cc (left):

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/new_tab_page_handler.cc:160: } else if
(histogram_name == "ExtendedNewTabPage.TimeToClickMostVisited") {
then this one can probably be deleted as well

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/ntp_resource_cache.cc (left):

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:432:
l10n_util::GetStringUTF16(IDS_NEW_TAB_RECENTLY_CLOSED));
On 2015/05/14 01:09:34, Dan Beam wrote:
> On 2015/05/14 00:09:45, Evan Stade wrote:
> > On 2015/05/13 23:18:01, Dan Beam wrote:
> > > these are all still used from the flapjacks menu
> > 
> > should probably consider renaming them/reorganizing the string file
> 
> they're all still valid ("1 tab", "## tabs" and "Recently closed" are all
still
> used and validly named)

IDS_NEW_TAB_FOO is the conventional name of a string that is used in the new tab
page, and strings used in the NTP are grouped together in the grd file.

[Dan Beam, 2015-05-14 19:56:47]

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/new_tab_page_handler.cc (left):

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/new_tab_page_handler.cc:160: } else if
(histogram_name == "ExtendedNewTabPage.TimeToClickMostVisited") {
On 2015/05/14 19:04:55, Evan Stade wrote:
> then this one can probably be deleted as well

there's a lot more code to go.  this CL is targeted.

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/ntp_resource_cache.cc (left):

https://codereview.chromium.org/1130753007/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:432:
l10n_util::GetStringUTF16(IDS_NEW_TAB_RECENTLY_CLOSED));
On 2015/05/14 19:04:55, Evan Stade wrote:
> On 2015/05/14 01:09:34, Dan Beam wrote:
> > On 2015/05/14 00:09:45, Evan Stade wrote:
> > > On 2015/05/13 23:18:01, Dan Beam wrote:
> > > > these are all still used from the flapjacks menu
> > > 
> > > should probably consider renaming them/reorganizing the string file
> > 
> > they're all still valid ("1 tab", "## tabs" and "Recently closed" are all
> still
> > used and validly named)
> 
> IDS_NEW_TAB_FOO is the conventional name of a string that is used in the new
tab
> page, and strings used in the NTP are grouped together in the grd file.

ah, the prefix. Done.

[Evan Stade, 2015-05-14 20:00:13]

lgtm

[Evan Stade, 2015-05-14 20:02:18]

https://codereview.chromium.org/1130753007/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1130753007/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:12481: +        <message
name="IDS_RECENTLY_CLOSED" desc="The header title of the Recent Tabs menu.">
nit: this isn't the most helpful description

"The section header for the Recently Closed section of the Recent Tabs menu."
(The other sections are tabs on other devices.)

[Dan Beam, 2015-05-14 20:48:25]

https://codereview.chromium.org/1130753007/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1130753007/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:12481: +        <message
name="IDS_RECENTLY_CLOSED" desc="The header title of the Recent Tabs menu.">
On 2015/05/14 20:02:17, Evan Stade wrote:
> nit: this isn't the most helpful description
> 
> "The section header for the Recently Closed section of the Recent Tabs menu."
> (The other sections are tabs on other devices.)

Done.

[Dan Beam, 2015-05-14 20:49:40]

dbeam@chromium.org changed reviewers:
+ thestig@chromium.org

[Dan Beam, 2015-05-14 20:49:40]

+thestig@ for chrome/ stamp

[Dan Beam, 2015-05-14 20:50:06]

thestig@: specifically

Missing LGTM from an OWNER for these files:
    chrome/browser/jumplist_win.cc
    chrome/browser/ui/cocoa/history_menu_bridge.mm
    chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc
    chrome/browser/ui/views/frame/global_menu_bar_x11.cc

[Lei Zhang, 2015-05-14 20:52:29]

On 2015/05/14 20:50:06, Dan Beam wrote:
> thestig@: specifically
> 
> Missing LGTM from an OWNER for these files:
>     chrome/browser/jumplist_win.cc
>     chrome/browser/ui/cocoa/history_menu_bridge.mm
>     chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc
>     chrome/browser/ui/views/frame/global_menu_bar_x11.cc

those files lgtm

[Dan Beam, 2015-05-14 21:06:36]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2015-05-14 21:06:36]

The patchset sent to the CQ was uploaded after l-g-t-m from estade@chromium.org
Link to the patchset: https://codereview.chromium.org/1130753007/#ps60001
(title: "estade@ review")

[commit-bot: I haz the power, 2015-05-14 21:08:24]

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

[commit-bot: I haz the power, 2015-05-14 21:37:35]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-05-14 21:38:24]

Patchset 3 (id:??) landed as
https://crrev.com/7894b5557c317e6302d3dde13b28dec8c3798d46
Cr-Commit-Position: refs/heads/master@{#329937}