Move shared file type images into chrome://theme/ scheme

Move shared file type images into chrome://theme/ scheme

These images are used by WebUI code (which uses 2x/ subdirectories for hi-dpi) and by C++ code (which uses the default_###_percent scheme). Move them to chrome/app/theme so that scaling will work in C++ code.

drive.png was unmentioned in the grd file. I added IDR_FILE_MANAGER_IMG_FILETYPE_DRIVE. Some uses of the images in CSS/JS use only the 1x versions. I left these as-is.


BUG=138965
TEST=Look at file manager and chrome://downloads in Linux chromeos build with --force-device-scale-factor=[12]


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

[zel, 2012-09-14 02:09:38]

try this to fix your theme problems - in extension_ui.cc, add the following #if
block:

ExtensionsUI::ExtensionsUI(content::WebUI* web_ui) : WebUIController(web_ui) {
 Profile* profile = Profile::FromWebUI(web_ui);
 ChromeWebUIDataSource* source = CreateExtensionsHTMLSource();
 ChromeURLDataManager::AddDataSource(profile, source);
 ChromeURLDataManager::AddDataSource(profile, new SharedResourcesDataSource());
#if defined(OS_CHROMEOS)  
 ChromeURLDataManager::AddDataSource(profile, new ThemeSource());
#endif
...

[zel, 2012-09-14 16:28:32]

Please ignore my previous comment. Here are all changes that you are really
missing in your CL:


https://codereview.chromium.org/10913275/diff/1/chrome/common/extensions/exte...
https://codereview.chromium.org/10913275/diff/1/chrome/common/url_constants.h
https://codereview.chromium.org/10913275/diff/1/chrome/common/url_constants.cc
https://codereview.chromium.org/10913275/diff/1/chrome/browser/extensions/ext...



On Thu, Sep 13, 2012 at 7:09 PM, <zelidrag@chromium.org> wrote:

> try this to fix your theme problems - in extension_ui.cc, add the
> following #if
> block:
>
> ExtensionsUI::ExtensionsUI(**content::WebUI* web_ui) :
> WebUIController(web_ui) {
>  Profile* profile = Profile::FromWebUI(web_ui);
>  ChromeWebUIDataSource* source = CreateExtensionsHTMLSource();
>  ChromeURLDataManager::**AddDataSource(profile, source);
>  ChromeURLDataManager::**AddDataSource(profile, new
> SharedResourcesDataSource());
> #if defined(OS_CHROMEOS)
>  ChromeURLDataManager::**AddDataSource(profile, new ThemeSource());
> #endif
> ...
>
>
https://codereview.chromium.**org/10916305/<https://codereview.chromium.org/1...
>

[flackr, 2012-09-14 16:33:46]

lgtm

[benrg, 2012-09-14 17:10:59]

Ben, requesting owner approval.

[benrg, 2012-09-14 17:11:39]

Ben, requesting owner approval.

[Ben Goodger (Google), 2012-09-14 17:12:29]

lgtm

[zel, 2012-09-14 17:15:03]

+asargent for extensions part
+kaznacheev for file manager part

LGTM

[Vladislav Kaznacheev, 2012-09-14 17:56:03]

https://chromiumcodereview.appspot.com/10916305/diff/7001/chrome/browser/reso...
File chrome/browser/resources/file_manager/video_player.html (right):

https://chromiumcodereview.appspot.com/10916305/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/video_player.html:13: <link rel="icon"
type="image/png" href="chrome://theme/IDR_FILE_MANAGER_IMG_FILETYPE_VIDEO">
Please make a similar change to file_manager/mediaplayer.html

[benrg, 2012-09-14 18:21:18]

https://chromiumcodereview.appspot.com/10916305/diff/7001/chrome/browser/reso...
File chrome/browser/resources/file_manager/video_player.html (right):

https://chromiumcodereview.appspot.com/10916305/diff/7001/chrome/browser/reso...
chrome/browser/resources/file_manager/video_player.html:13: <link rel="icon"
type="image/png" href="chrome://theme/IDR_FILE_MANAGER_IMG_FILETYPE_VIDEO">
On 2012/09/14 17:56:04, Vladislav Kaznacheev wrote:
> Please make a similar change to file_manager/mediaplayer.html

This CL only moves the images in file_types/, so there's no chrome://theme/ URL
for media/audio_player.png.

[Vladislav Kaznacheev, 2012-09-14 19:22:25]

I am sorry, I forgot we have a dedicated icon for the Audio Player. 

LGTM then.

On 2012/09/14 18:21:18, benrg wrote:
>
https://chromiumcodereview.appspot.com/10916305/diff/7001/chrome/browser/reso...
> File chrome/browser/resources/file_manager/video_player.html (right):
> 
>
https://chromiumcodereview.appspot.com/10916305/diff/7001/chrome/browser/reso...
> chrome/browser/resources/file_manager/video_player.html:13: <link rel="icon"
> type="image/png" href="chrome://theme/IDR_FILE_MANAGER_IMG_FILETYPE_VIDEO">
> On 2012/09/14 17:56:04, Vladislav Kaznacheev wrote:
> > Please make a similar change to file_manager/mediaplayer.html
> 
> This CL only moves the images in file_types/, so there's no chrome://theme/
URL
> for media/audio_player.png.

[asargent_no_longer_on_chrome, 2012-09-14 22:01:01]

https://codereview.chromium.org/10916305/diff/7001/chrome/common/extensions/e...
File chrome/common/extensions/extension.cc (right):

https://codereview.chromium.org/10916305/diff/7001/chrome/common/extensions/e...
chrome/common/extensions/extension.cc:3494: url.host() !=
chrome::kChromeUIThemeHost &&
Are you sure you want to let non-component extensions load chrome://theme
resource urls?

[zel, 2012-09-14 22:29:25]

https://codereview.chromium.org/10916305/diff/7001/chrome/common/extensions/e...
File chrome/common/extensions/extension.cc (right):

https://codereview.chromium.org/10916305/diff/7001/chrome/common/extensions/e...
chrome/common/extensions/extension.cc:3494: url.host() !=
chrome::kChromeUIThemeHost &&
On 2012/09/14 22:01:01, Antony Sargent wrote:
> Are you sure you want to let non-component extensions load chrome://theme
> resource urls?

No, we don't, I misinterpreted this if statement. Ben, please remove this added
line.

[benrg, 2012-09-14 22:38:15]

asargent, PTAL

https://codereview.chromium.org/10916305/diff/7001/chrome/common/extensions/e...
File chrome/common/extensions/extension.cc (right):

https://codereview.chromium.org/10916305/diff/7001/chrome/common/extensions/e...
chrome/common/extensions/extension.cc:3494: url.host() !=
chrome::kChromeUIThemeHost &&
On 2012/09/14 22:29:25, zel wrote:
> On 2012/09/14 22:01:01, Antony Sargent wrote:
> > Are you sure you want to let non-component extensions load chrome://theme
> > resource urls?
> 
> No, we don't, I misinterpreted this if statement. Ben, please remove this
added
> line.

Done.

[asargent_no_longer_on_chrome, 2012-09-14 23:29:17]

extension_service.cc LGTM

[commit-bot: I haz the power, 2012-09-15 00:05:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benrg@chromium.org/10916305/9005

[commit-bot: I haz the power, 2012-09-15 01:19:27]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-09-15 05:29:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benrg@chromium.org/10916305/9005

[commit-bot: I haz the power, 2012-09-15 06:01:36]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-09-15 17:40:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benrg@chromium.org/10916305/9005

[commit-bot: I haz the power, 2012-09-15 20:16:17]

Change committed as 157013