ntp: remove ntp3 resources

ntp: remove ntp3 resources

BUG=97706, 76706
TEST=manual

[Rick Byers, 2011-09-26 16:05:26]

LGTM
It's a little unfortunate that we'll no longer be able to enable NTP4 with a
flag on TOUCH_UI builds, but when we get time to just switch over it hopefully
will be a small enough job that a flag won't be necessary (but we can always
re-add one temporarily).

Let me know if you won't get a chance to validate that this doesn't break the
NTP in TOUCH_UI builds and I can test it out.

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/recently_closed.css (right):

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/recently_closed.css:74: background-image:
url('./images/closed_window.png');
curious: why the "./"?  Other WebUI components appear to just do
"url('images/closed_window.png')"

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/ui/webui/ntp/...
File chrome/browser/ui/webui/ntp/ntp_resource_cache.cc (right):

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/ui/webui/ntp/...
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:428: #if defined(TOUCH_UI)
Ugh - sure will be nice to move touch_ui to NTP4 and eliminate this too.  But
this is fine for now.

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/ui/webui/ntp/...
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:587: new_tab_css_ =
base::RefCountedString::TakeString(&css_string);
I know touch_ntp doesn't rely on any of the theme CSS rules, but does the
TOUCH_UI build deal with new_tab_css_ being NULL  alright?  You could always
just assign an empty string instead if NULL is likely to be problematic.

[csilv, 2011-09-26 22:25:47]

LGTM, see comments.

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/history/top_s...
File chrome/browser/history/top_sites.cc (left):

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/history/top_s...
chrome/browser/history/top_sites.cc:63: // Same as above, but for NTP4.
TODO(estade): remove the above.
It looks like you removed the wrong lines here.

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/recently_closed.css (right):

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/recently_closed.css:89: -webkit-mask-image:
url('./images/ntp4_disclosure_triangle_mask.png');
Could we consider removing the "ntp4_" prefix to the filename?  It's the only
file in the ntp4 directory with that prefix, and since it's already in the ntp4
directory, not really needed.

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/ui/webui/ntp/...
File chrome/browser/ui/webui/ntp/app_launcher_handler.cc (right):

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/ui/webui/ntp/...
chrome/browser/ui/webui/ntp/app_launcher_handler.cc:100: // Don't include the
WebStore and the Cloud Print app.
You can remove this comment line.

[Evan Stade, 2011-09-26 22:32:05]

update: I will have to back out the bookmark bar related changes. Best to keep
it easy to resort to showing the bar, as we did temporarily today.

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/history/top_s...
File chrome/browser/history/top_sites.cc (left):

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/history/top_s...
chrome/browser/history/top_sites.cc:63: // Same as above, but for NTP4.
TODO(estade): remove the above.
On 2011/09/26 22:25:47, csilv wrote:
> It looks like you removed the wrong lines here.

aye, but I didn't. We're using 16x16 for now.

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/recently_closed.css (right):

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/recently_closed.css:74: background-image:
url('./images/closed_window.png');
On 2011/09/26 16:05:26, Rick Byers wrote:
> curious: why the "./"?  Other WebUI components appear to just do
> "url('images/closed_window.png')"

I was matching L89. It's probably not necessary in either place.

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/recently_closed.css:89: -webkit-mask-image:
url('./images/ntp4_disclosure_triangle_mask.png');
On 2011/09/26 22:25:47, csilv wrote:
> Could we consider removing the "ntp4_" prefix to the filename?  It's the only
> file in the ntp4 directory with that prefix, and since it's already in the
ntp4
> directory, not really needed.

yes

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/ui/webui/ntp/...
File chrome/browser/ui/webui/ntp/app_launcher_handler.cc (right):

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/ui/webui/ntp/...
chrome/browser/ui/webui/ntp/app_launcher_handler.cc:100: // Don't include the
WebStore and the Cloud Print app.
On 2011/09/26 22:25:47, csilv wrote:
> You can remove this comment line.

yea

[csilv, 2011-09-26 22:36:46]

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/history/top_s...
File chrome/browser/history/top_sites.cc (left):

http://codereview.chromium.org/8036002/diff/2001/chrome/browser/history/top_s...
chrome/browser/history/top_sites.cc:63: // Same as above, but for NTP4.
TODO(estade): remove the above.
On 2011/09/26 22:32:05, Evan Stade wrote:
> On 2011/09/26 22:25:47, csilv wrote:
> > It looks like you removed the wrong lines here.
> 
> aye, but I didn't. We're using 16x16 for now.

Ah right of course.