Set a reasonable icon for the media galleries scan result dialog folder viewer.

Set a reasonable icon for the media galleries scan result dialog folder viewer.

BUG=334309, 161119
NOTRY=true

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

[vandebo (ex-Chrome), 2014-01-31 21:54:07]

oshima: theme_resources
groby: cocoa
sky: views

[groby-ooo-7-16, 2014-01-31 22:36:42]

Cocoa LGTM

[oshima, 2014-02-03 20:14:14]

Did UX folks agreed and are aware that we're using the folder (originally)
designed for cros? I just want to void situation where updating png files
believing it's cros only can cause issues on other places.

If so, please move the png file from cros/ to common/ directory.

[vandebo (ex-Chrome), 2014-02-05 00:04:18]

On 2014/02/03 20:14:14, oshima wrote:
> Did UX folks agreed and are aware that we're using the folder (originally)
> designed for cros? I just want to void situation where updating png files
> believing it's cros only can cause issues on other places.
> 
> If so, please move the png file from cros/ to common/ directory.

UX suggested this asset, they didn't not suggest sharing it across changes. 
I've moved the file as you suggested.

[oshima, 2014-02-07 20:12:55]

+yoshiki

https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
File chrome/app/theme/theme_resources.grd (right):

https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
chrome/app/theme/theme_resources.grd:235: <structure type="chrome_scaled_image"
name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER" file="common/folder.png" />
I actually wonder if this (and some of files in cros/file_types) are really used
in filemanager.

(https://code.google.com/p/chromium/codesearch#search/&q=IDR_FILE_MANAGER_IMG_...)

yoshiki, do you know if we need them all?

[vandebo (ex-Chrome), 2014-02-08 00:47:44]

https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
File chrome/app/theme/theme_resources.grd (right):

https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
chrome/app/theme/theme_resources.grd:235: <structure type="chrome_scaled_image"
name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER" file="common/folder.png" />
On 2014/02/07 20:12:55, oshima wrote:
> I actually wonder if this (and some of files in cros/file_types) are really
used
> in filemanager.
> 
>
(https://code.google.com/p/chromium/codesearch#search/&q=IDR_FILE_MANAGER_IMG_...)
> 
> yoshiki, do you know if we need them all?

That sound out of scope for this CL.  Maybe yoshiki can do a follow up CL?

[oshima, 2014-02-08 00:53:28]

Using the name IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER for non filemanager
code
is not good idea. I think we should just rename it to IDR_FILE_FOLDER. If
this is not used, then you can just rename and
use it without worrying about the breakage.


On Fri, Feb 7, 2014 at 4:47 PM, <vandebo@chromium.org> wrote:

>
> https://codereview.chromium.org/130263007/diff/70001/
> chrome/app/theme/theme_resources.grd
> File chrome/app/theme/theme_resources.grd (right):
>
> https://codereview.chromium.org/130263007/diff/70001/
> chrome/app/theme/theme_resources.grd#newcode235
> chrome/app/theme/theme_resources.grd:235: <structure
> type="chrome_scaled_image" name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER"
> file="common/folder.png" />
> On 2014/02/07 20:12:55, oshima wrote:
>
>> I actually wonder if this (and some of files in cros/file_types) are
>>
> really used
>
>> in filemanager.
>>
>
>
> (https://code.google.com/p/chromium/codesearch#search/&q=
> IDR_FILE_MANAGER_IMG_FILETYPE&sq=package:chromium&type=cs)
>
>  yoshiki, do you know if we need them all?
>>
>
> That sound out of scope for this CL.  Maybe yoshiki can do a follow up
> CL?
>
> https://codereview.chromium.org/130263007/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vandebo (ex-Chrome), 2014-02-08 00:56:31]

On 2014/02/08 00:53:28, oshima wrote:
> Using the name IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER for non filemanager
> code
> is not good idea. I think we should just rename it to IDR_FILE_FOLDER. If
> this is not used, then you can just rename and
> use it without worrying about the breakage.

Agreed, but I'm not changing the name, and moved it to common to help prevent
breakage.
You suggested that many of these resources may not be used in file manager,
which sounds like a separate cleanup effort is needed there.

[oshima, 2014-02-08 01:07:09]

On 2014/02/08 00:56:31, vandebo wrote:
> On 2014/02/08 00:53:28, oshima wrote:
> > Using the name IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER for non filemanager
> > code
> > is not good idea. I think we should just rename it to IDR_FILE_FOLDER. If
> > this is not used, then you can just rename and
> > use it without worrying about the breakage.
> 
> Agreed, but I'm not changing the name, and moved it to common to help prevent
> breakage.
> You suggested that many of these resources may not be used in file manager,
> which sounds like a separate cleanup effort is needed there.

I'm not asking you to remove them. I'm against using IDR_FILEMANAGER_XXX name in
media_galleries file,
and I'm asking him to know if it's safe to rename to IDR_FILE_FOLDER. (If this
IDR is no longer used,
we can rename it without changing other files)

[vandebo (ex-Chrome), 2014-02-10 17:12:15]

On 2014/02/07 20:12:55, oshima wrote:
> +yoshiki
> 
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> File chrome/app/theme/theme_resources.grd (right):
> 
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> chrome/app/theme/theme_resources.grd:235: <structure
type="chrome_scaled_image"
> name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER" file="common/folder.png" />
> I actually wonder if this (and some of files in cros/file_types) are really
used
> in filemanager.
> 
>
(https://code.google.com/p/chromium/codesearch#search/&q=IDR_FILE_MANAGER_IMG_...)
> 
> yoshiki, do you know if we need them all?

ping

[oshima, 2014-02-11 22:28:31]

On 2014/02/10 17:12:15, vandebo wrote:
> On 2014/02/07 20:12:55, oshima wrote:
> > +yoshiki
> > 
> >
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> > File chrome/app/theme/theme_resources.grd (right):
> > 
> >
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> > chrome/app/theme/theme_resources.grd:235: <structure
> type="chrome_scaled_image"
> > name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER" file="common/folder.png" />
> > I actually wonder if this (and some of files in cros/file_types) are really
> used
> > in filemanager.
> > 
> >
>
(https://code.google.com/p/chromium/codesearch#search/&q=IDR_FILE_MANAGER_IMG_...)
> > 
> > yoshiki, do you know if we need them all?
> 
> ping

I tried to reach him last night but he was not avail. I'll try again tonight and
if he didn't respond, I'll approve as the branch point is near.

[vandebo (ex-Chrome), 2014-02-11 22:31:16]

On 2014/02/11 22:28:31, oshima wrote:
> On 2014/02/10 17:12:15, vandebo wrote:
> > On 2014/02/07 20:12:55, oshima wrote:
> > > +yoshiki
> > > 
> > >
> >
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> > > File chrome/app/theme/theme_resources.grd (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> > > chrome/app/theme/theme_resources.grd:235: <structure
> > type="chrome_scaled_image"
> > > name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER" file="common/folder.png" />
> > > I actually wonder if this (and some of files in cros/file_types) are
really
> > used
> > > in filemanager.
> > > 
> > >
> >
>
(https://code.google.com/p/chromium/codesearch#search/&q=IDR_FILE_MANAGER_IMG_...)
> > > 
> > > yoshiki, do you know if we need them all?
> > 
> > ping
> 
> I tried to reach him last night but he was not avail. I'll try again tonight
and
> if he didn't respond, I'll approve as the branch point is near.

This dialog is behind a flag for M34, but I appreciate the help in getting it
committed.

[yoshiki, 2014-02-12 02:14:41]

On 2014/02/11 22:31:16, vandebo wrote:
> On 2014/02/11 22:28:31, oshima wrote:
> > On 2014/02/10 17:12:15, vandebo wrote:
> > > On 2014/02/07 20:12:55, oshima wrote:
> > > > +yoshiki
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> > > > File chrome/app/theme/theme_resources.grd (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> > > > chrome/app/theme/theme_resources.grd:235: <structure
> > > type="chrome_scaled_image"
> > > > name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER" file="common/folder.png" />
> > > > I actually wonder if this (and some of files in cros/file_types) are
> really
> > > used
> > > > in filemanager.
> > > > 
> > > >
> > >
> >
>
(https://code.google.com/p/chromium/codesearch#search/&q=IDR_FILE_MANAGER_IMG_...)
> > > > 
> > > > yoshiki, do you know if we need them all?
> > > 
> > > ping
> > 
> > I tried to reach him last night but he was not avail. I'll try again tonight
> and
> > if he didn't respond, I'll approve as the branch point is near.
> 
> This dialog is behind a flag for M34, but I appreciate the help in getting it
> committed.

Sorry for late, I was ooo until yesterday. I'll check it today.

[yoshiki, 2014-02-12 12:24:09]

https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
File chrome/app/theme/theme_resources.grd (right):

https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
chrome/app/theme/theme_resources.grd:235: <structure type="chrome_scaled_image"
name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER" file="common/folder.png" />
On 2014/02/08 00:47:44, vandebo wrote:
> On 2014/02/07 20:12:55, oshima wrote:
> > I actually wonder if this (and some of files in cros/file_types) are really
> used
> > in filemanager.
> > 
> >
>
(https://code.google.com/p/chromium/codesearch#search/&q=IDR_FILE_MANAGER_IMG_...)
> > 
> > yoshiki, do you know if we need them all?
> 
> That sound out of scope for this CL.  Maybe yoshiki can do a follow up CL?

This had been used by Files.app but doesn't use now. You can move the file
or/and rename the ID if you want.

[oshima, 2014-02-12 12:29:15]

On 2014/02/12 12:24:09, yoshiki wrote:
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> File chrome/app/theme/theme_resources.grd (right):
> 
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> chrome/app/theme/theme_resources.grd:235: <structure
type="chrome_scaled_image"
> name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER" file="common/folder.png" />
> On 2014/02/08 00:47:44, vandebo wrote:
> > On 2014/02/07 20:12:55, oshima wrote:
> > > I actually wonder if this (and some of files in cros/file_types) are
really
> > used
> > > in filemanager.
> > > 
> > >
> >
>
(https://code.google.com/p/chromium/codesearch#search/&q=IDR_FILE_MANAGER_IMG_...)
> > > 
> > > yoshiki, do you know if we need them all?
> > 
> > That sound out of scope for this CL.  Maybe yoshiki can do a follow up CL?
> 
> This had been used by Files.app but doesn't use now. You can move the file
> or/and rename the ID if you want.

Thanks. vandebo@, can you rename it to IDR_FILE_FOLDER?

yoshiki@, what about other files? Would you mind cleaning other files up?

[yoshiki, 2014-02-12 13:04:50]

On 2014/02/12 12:29:15, oshima wrote:
> On 2014/02/12 12:24:09, yoshiki wrote:
> >
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> > File chrome/app/theme/theme_resources.grd (right):
> > 
> >
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> > chrome/app/theme/theme_resources.grd:235: <structure
> type="chrome_scaled_image"
> > name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER" file="common/folder.png" />
> > On 2014/02/08 00:47:44, vandebo wrote:
> > > On 2014/02/07 20:12:55, oshima wrote:
> > > > I actually wonder if this (and some of files in cros/file_types) are
> really
> > > used
> > > > in filemanager.
> > > > 
> > > >
> > >
> >
>
(https://code.google.com/p/chromium/codesearch#search/&q=IDR_FILE_MANAGER_IMG_...)
> > > > 
> > > > yoshiki, do you know if we need them all?
> > > 
> > > That sound out of scope for this CL.  Maybe yoshiki can do a follow up CL?
> > 
> > This had been used by Files.app but doesn't use now. You can move the file
> > or/and rename the ID if you want.
> 
> Thanks. vandebo@, can you rename it to IDR_FILE_FOLDER?
> 
> yoshiki@, what about other files? Would you mind cleaning other files up?

I'll do separately: http://crbug.com/343127.

[oshima, 2014-02-12 13:59:42]

On 2014/02/12 13:04:50, yoshiki wrote:
> On 2014/02/12 12:29:15, oshima wrote:
> > On 2014/02/12 12:24:09, yoshiki wrote:
> > >
> >
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> > > File chrome/app/theme/theme_resources.grd (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/130263007/diff/70001/chrome/app/theme/theme_r...
> > > chrome/app/theme/theme_resources.grd:235: <structure
> > type="chrome_scaled_image"
> > > name="IDR_FILE_MANAGER_IMG_FILETYPE_FOLDER" file="common/folder.png" />
> > > On 2014/02/08 00:47:44, vandebo wrote:
> > > > On 2014/02/07 20:12:55, oshima wrote:
> > > > > I actually wonder if this (and some of files in cros/file_types) are
> > really
> > > > used
> > > > > in filemanager.
> > > > > 
> > > > >
> > > >
> > >
> >
>
(https://code.google.com/p/chromium/codesearch#search/&q=IDR_FILE_MANAGER_IMG_...)
> > > > > 
> > > > > yoshiki, do you know if we need them all?
> > > > 
> > > > That sound out of scope for this CL.  Maybe yoshiki can do a follow up
CL?
> > > 
> > > This had been used by Files.app but doesn't use now. You can move the file
> > > or/and rename the ID if you want.
> > 
> > Thanks. vandebo@, can you rename it to IDR_FILE_FOLDER?
> > 
> > yoshiki@, what about other files? Would you mind cleaning other files up?
> 
> I'll do separately: http://crbug.com/343127.

awesome, thank you!

[vandebo (ex-Chrome), 2014-02-12 18:54:09]

sky: please review views

On 2014/02/12 12:29:15, oshima wrote:
> Thanks. vandebo@, can you rename it to IDR_FILE_FOLDER?

Done.

[oshima, 2014-02-12 20:15:35]

lgtm

and thank you for your patience.

[sky, 2014-02-12 21:26:54]

LGTM

[vandebo (ex-Chrome), 2014-02-12 21:34:31]

The CQ bit was checked by vandebo@chromium.org

[commit-bot: I haz the power, 2014-02-12 21:35:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vandebo@chromium.org/130263007/260001

[commit-bot: I haz the power, 2014-02-12 22:08:34]

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

[commit-bot: I haz the power, 2014-02-12 22:08:34]

Retried try job too often on linux_rel for step(s) cacheinvalidation_unittests,
cc_unittests, check_deps, check_deps2git, chromedriver_unittests,
components_unittests, content_browsertests, content_unittests, crypto_unittests,
google_apis_unittests, gpu_unittests, ipc_tests, jingle_unittests,
media_unittests, nacl_integration, ppapi_unittests, printing_unittests,
remoting_unittests, sandbox_linux_unittests, sql_unittests,
sync_integration_tests, sync_unit_tests, telemetry_perf_unittests,
telemetry_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[vandebo (ex-Chrome), 2014-02-13 21:43:03]

The CQ bit was checked by vandebo@chromium.org

[commit-bot: I haz the power, 2014-02-13 21:48:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vandebo@chromium.org/130263007/260001

[commit-bot: I haz the power, 2014-02-13 22:45:39]

Change committed as 251163