Removing base::ThreadRestrictions::ScopedAllowIO from icon_manager_linux.cc

Removing base::ThreadRestrictions::ScopedAllowIO from icon_manager_linux.cc

Changed cache to map file paths to icons rather than by group id. Renamed
GetGroupIDFromFilepath to reflect that it can potentially do file io.

BUG=72740
TEST=Bring up downloads ui on linux and file icons still show up.

R=rdsmith@chromium.org


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

[Randy Smith (Not in Mondays), 2013-02-11 00:45:58]

Downloads code LGTM.

[Robert Sesek, 2013-02-11 20:52:22]

https://codereview.chromium.org/12211049/diff/1/chrome/browser/icon_manager.h
File chrome/browser/icon_manager.h (right):

https://codereview.chromium.org/12211049/diff/1/chrome/browser/icon_manager.h...
chrome/browser/icon_manager.h:94: CacheKey(const base::FilePath& file_name,
IconLoader::IconSize size);
I think this change will result in many more cache misses, and therefore more
icon reading and icon storing. Clients likely won't look up the icon for the
same file multiple times, but may look up icons for the same kind of file.

Maybe the IconLoader::Delegate should be extended with a |gfx::Image*
CachedIconWithGroupID(const IconGroupID&)| method, which would involve a series
of message passes:

Client calls LoadIcon [UI thread] -> ReadGroup [FILE thread] -> Check Cache [UI]
-> Load Icon if Necessary [FILE] -> Run Callback [UI]

If that new delegate method returns non-NULL, the icon isn't loaded again and
the callback is just run with the cache result.

[Robert Sesek, 2013-02-11 21:09:43]

On 2013/02/11 20:52:22, rsesek wrote:
> https://codereview.chromium.org/12211049/diff/1/chrome/browser/icon_manager.h
> File chrome/browser/icon_manager.h (right):
> 
>
https://codereview.chromium.org/12211049/diff/1/chrome/browser/icon_manager.h...
> chrome/browser/icon_manager.h:94: CacheKey(const base::FilePath& file_name,
> IconLoader::IconSize size);
> I think this change will result in many more cache misses, and therefore more
> icon reading and icon storing. Clients likely won't look up the icon for the
> same file multiple times, but may look up icons for the same kind of file.
> 
> Maybe the IconLoader::Delegate should be extended with a |gfx::Image*
> CachedIconWithGroupID(const IconGroupID&)| method, which would involve a
series
> of message passes:
> 
> Client calls LoadIcon [UI thread] -> ReadGroup [FILE thread] -> Check Cache
[UI]
> -> Load Icon if Necessary [FILE] -> Run Callback [UI]
> 
> If that new delegate method returns non-NULL, the icon isn't loaded again and
> the callback is just run with the cache result.

However this approach is a bit wonky because gfx::Image isn't threadsafe. And
while this wouldn't technically violate that principle, it makes it easier to.
Another option would be to split icon loading into a two-phase system: one to
read the group type and one to read the actual icon. IconManager would run this
two-phase system. The flow would be the same as outlined above, but rather than
having the IconLoader ask for a cached icon, the IconLoader would just call back
to the delegate via OnIconGroupLoaded(FilePath, IconGroupID) and then the
IconManager could look up in the cache that way.

[shatch, 2013-02-12 00:01:27]

New snapshot loaded.

Added 2-phase icon loading as described by rsesek.

[Robert Sesek, 2013-02-12 16:49:25]

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader.cc
File chrome/browser/icon_loader.cc (right):

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader...
chrome/browser/icon_loader.cc:38: IconGroupID* group = new IconGroupID();
Who owns this? When is it deleted? Can't this just be a member?

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader.h
File chrome/browser/icon_loader.h (right):

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader...
chrome/browser/icon_loader.h:53: // is read. If the icon is already cached, this
method returns true.
What does this class do if the method returns true?

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader...
chrome/browser/icon_loader.h:54: virtual bool OnGroupLoaded(IconLoader* source,
Since this happens before OnImageLoaded, put it before.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader...
chrome/browser/icon_loader.h:74: void ReadGroup(IconGroupID* group);
Same comment about ordering.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
File chrome/browser/icon_manager.cc (right):

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.cc:42: IconLoader::IconSize size) {
nit: this isn't wrapped correctly. It should either align with 'c' in const or
just be indented 4 spaces. Same on line 51.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.cc:104: return true;
Don't you also need to IconLoader::Release() if this happens since OnImageLoaded
won't be called?

But going further, could you rewrite OnGroupLoaded to just call through to
OnImageLoaded so you don't have to duplicate so much bookkeeping logic?

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manager.h
File chrome/browser/icon_manager.h (right):

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.h:67: gfx::Image* LookupIconFromGroup(const
IconGroupID& group,
Does this need to be public?

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.h:91: 
nit: no blank line needed

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.h:113: typedef std::map<FilePath, IconGroupID>
GroupMap;
I'm not sure how much of a benefit this cache mapping will provide. My guess
would be not much, but that's just a guess.

[shatch, 2013-02-13 16:14:52]

New snapshot uploaded.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader.cc
File chrome/browser/icon_loader.cc (right):

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader...
chrome/browser/icon_loader.cc:38: IconGroupID* group = new IconGroupID();
On 2013/02/12 16:49:25, rsesek wrote:
> Who owns this? When is it deleted? Can't this just be a member?

Done.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader.h
File chrome/browser/icon_loader.h (right):

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader...
chrome/browser/icon_loader.h:53: // is read. If the icon is already cached, this
method returns true.
On 2013/02/12 16:49:25, rsesek wrote:
> What does this class do if the method returns true?

Done.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader...
chrome/browser/icon_loader.h:54: virtual bool OnGroupLoaded(IconLoader* source,
On 2013/02/12 16:49:25, rsesek wrote:
> Since this happens before OnImageLoaded, put it before.

Done.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_loader...
chrome/browser/icon_loader.h:74: void ReadGroup(IconGroupID* group);
On 2013/02/12 16:49:25, rsesek wrote:
> Same comment about ordering.

Done.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
File chrome/browser/icon_manager.cc (right):

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.cc:42: IconLoader::IconSize size) {
On 2013/02/12 16:49:25, rsesek wrote:
> nit: this isn't wrapped correctly. It should either align with 'c' in const or
> just be indented 4 spaces. Same on line 51.

Done.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.cc:104: return true;
On 2013/02/12 16:49:25, rsesek wrote:
> Don't you also need to IconLoader::Release() if this happens since
OnImageLoaded
> won't be called?
> 
> But going further, could you rewrite OnGroupLoaded to just call through to
> OnImageLoaded so you don't have to duplicate so much bookkeeping logic?

Done.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manager.h
File chrome/browser/icon_manager.h (right):

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.h:67: gfx::Image* LookupIconFromGroup(const
IconGroupID& group,
On 2013/02/12 16:49:25, rsesek wrote:
> Does this need to be public?

Doesn't need to be, but I made it public in case anybody in the future wants
access to it. I can make it private if it's an issue.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.h:91: 
On 2013/02/12 16:49:25, rsesek wrote:
> nit: no blank line needed

Done.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.h:113: typedef std::map<FilePath, IconGroupID>
GroupMap;
On 2013/02/12 16:49:25, rsesek wrote:
> I'm not sure how much of a benefit this cache mapping will provide. My guess
> would be not much, but that's just a guess.

From what I could see, it got used when you did things like drag. The same
filepath as the download would be used.

[Robert Sesek, 2013-02-13 17:33:04]

Just a few more comments.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manager.h
File chrome/browser/icon_manager.h (right):

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.h:67: gfx::Image* LookupIconFromGroup(const
IconGroupID& group,
On 2013/02/13 16:14:52, shatch wrote:
> On 2013/02/12 16:49:25, rsesek wrote:
> > Does this need to be public?
> 
> Doesn't need to be, but I made it public in case anybody in the future wants
> access to it. I can make it private if it's an issue.

I prefer a minimally exposed interface. But your call.

https://codereview.chromium.org/12211049/diff/4002/chrome/browser/icon_manage...
chrome/browser/icon_manager.h:113: typedef std::map<FilePath, IconGroupID>
GroupMap;
On 2013/02/13 16:14:52, shatch wrote:
> On 2013/02/12 16:49:25, rsesek wrote:
> > I'm not sure how much of a benefit this cache mapping will provide. My guess
> > would be not much, but that's just a guess.
> 
> From what I could see, it got used when you did things like drag. The same
> filepath as the download would be used.

Ah, good point.

https://codereview.chromium.org/12211049/diff/14001/chrome/browser/icon_loader.h
File chrome/browser/icon_loader.h (right):

https://codereview.chromium.org/12211049/diff/14001/chrome/browser/icon_loade...
chrome/browser/icon_loader.h:46: // is read. If the icon is already cached, this
method will call and return
this method will -> this method SHOULD

https://codereview.chromium.org/12211049/diff/14001/chrome/browser/icon_loade...
chrome/browser/icon_loader.h:48: // this method are expected to skip the
subsequent call to OnImageLoaded
I don't understand this sentence.

https://codereview.chromium.org/12211049/diff/14001/chrome/browser/icon_manag...
File chrome/browser/icon_manager.cc (right):

https://codereview.chromium.org/12211049/diff/14001/chrome/browser/icon_manag...
chrome/browser/icon_manager.cc:84: // Look up our client state.
This comment doesn't add much value. Remove it and the blank line on 83.

https://codereview.chromium.org/12211049/diff/14001/chrome/browser/icon_manag...
chrome/browser/icon_manager.cc:90: const ClientRequest& client_request =
rit->second;
nit: you only consult this on line 92, so you can probably just do
rit->second.size.

[shatch, 2013-02-21 20:02:06]

New snapshot uploaded.

https://chromiumcodereview.appspot.com/12211049/diff/4002/chrome/browser/icon...
File chrome/browser/icon_manager.h (right):

https://chromiumcodereview.appspot.com/12211049/diff/4002/chrome/browser/icon...
chrome/browser/icon_manager.h:67: gfx::Image* LookupIconFromGroup(const
IconGroupID& group,
On 2013/02/13 17:33:05, rsesek wrote:
> On 2013/02/13 16:14:52, shatch wrote:
> > On 2013/02/12 16:49:25, rsesek wrote:
> > > Does this need to be public?
> > 
> > Doesn't need to be, but I made it public in case anybody in the future wants
> > access to it. I can make it private if it's an issue.
> 
> I prefer a minimally exposed interface. But your call.

Done.

https://chromiumcodereview.appspot.com/12211049/diff/14001/chrome/browser/ico...
File chrome/browser/icon_loader.h (right):

https://chromiumcodereview.appspot.com/12211049/diff/14001/chrome/browser/ico...
chrome/browser/icon_loader.h:46: // is read. If the icon is already cached, this
method will call and return
On 2013/02/13 17:33:05, rsesek wrote:
> this method will -> this method SHOULD

Done.

https://chromiumcodereview.appspot.com/12211049/diff/14001/chrome/browser/ico...
chrome/browser/icon_loader.h:48: // this method are expected to skip the
subsequent call to OnImageLoaded
On 2013/02/13 17:33:05, rsesek wrote:
> I don't understand this sentence.

Done.

https://chromiumcodereview.appspot.com/12211049/diff/14001/chrome/browser/ico...
File chrome/browser/icon_manager.cc (right):

https://chromiumcodereview.appspot.com/12211049/diff/14001/chrome/browser/ico...
chrome/browser/icon_manager.cc:84: // Look up our client state.
On 2013/02/13 17:33:05, rsesek wrote:
> This comment doesn't add much value. Remove it and the blank line on 83.

Done.

https://chromiumcodereview.appspot.com/12211049/diff/14001/chrome/browser/ico...
chrome/browser/icon_manager.cc:90: const ClientRequest& client_request =
rit->second;
On 2013/02/13 17:33:05, rsesek wrote:
> nit: you only consult this on line 92, so you can probably just do
> rit->second.size.

Done.

[Robert Sesek, 2013-02-21 21:07:10]

I lied. One last cleanup idea.

https://chromiumcodereview.appspot.com/12211049/diff/25002/chrome/browser/ico...
File chrome/browser/icon_manager.h (right):

https://chromiumcodereview.appspot.com/12211049/diff/25002/chrome/browser/ico...
chrome/browser/icon_manager.h:94: static IconGroupID
ReadGroupIDFromFilepath(const base::FilePath& path);
Actually, why can't this now be implemented in icon_loader_[platform].xx -- just
like IconLoader::ReadIcon?

[Robert Sesek, 2013-03-07 15:34:06]

Just checking in on this -- it's good cleanup and it's almost good to go.

[shatch, 2013-03-08 18:34:57]

Sorry, let this change slip. New snapshot uploaded.

https://codereview.chromium.org/12211049/diff/25002/chrome/browser/icon_manag...
File chrome/browser/icon_manager.h (right):

https://codereview.chromium.org/12211049/diff/25002/chrome/browser/icon_manag...
chrome/browser/icon_manager.h:94: static IconGroupID
ReadGroupIDFromFilepath(const base::FilePath& path);
On 2013/02/21 21:07:10, rsesek wrote:
> Actually, why can't this now be implemented in icon_loader_[platform].xx --
just
> like IconLoader::ReadIcon?

Done.

[Robert Sesek, 2013-03-08 18:41:26]

https://codereview.chromium.org/12211049/diff/25002/chrome/browser/icon_manag...
File chrome/browser/icon_manager_android.cc (right):

https://codereview.chromium.org/12211049/diff/25002/chrome/browser/icon_manag...
chrome/browser/icon_manager_android.cc:1: // Copyright (c) 2012 The Chromium
Authors. All rights reserved.
These files aren't coming up as 'D' in the patch set. Not sure why…

https://codereview.chromium.org/12211049/diff/39001/chrome/browser/icon_loade...
File chrome/browser/icon_loader_android.cc (right):

https://codereview.chromium.org/12211049/diff/39001/chrome/browser/icon_loade...
chrome/browser/icon_loader_android.cc:8: IconGroupID
IconLoader::ReadGroupIDFromFilepath(
Above static member functions, we generally put a "// static" comment. Same
applies to all files.

https://codereview.chromium.org/12211049/diff/39001/chrome/browser/icon_loade...
File chrome/browser/icon_loader_chromeos.cc (right):

https://codereview.chromium.org/12211049/diff/39001/chrome/browser/icon_loade...
chrome/browser/icon_loader_chromeos.cc:12: #include "base/file_path.h"
This file is now in base/files/

[shatch, 2013-03-09 01:05:52]

On 2013/03/08 18:41:26, rsesek wrote:
>
https://codereview.chromium.org/12211049/diff/25002/chrome/browser/icon_manag...
> File chrome/browser/icon_manager_android.cc (right):
> 
>
https://codereview.chromium.org/12211049/diff/25002/chrome/browser/icon_manag...
> chrome/browser/icon_manager_android.cc:1: // Copyright (c) 2012 The Chromium
> Authors. All rights reserved.
> These files aren't coming up as 'D' in the patch set. Not sure why…
> 

I'm a bit confused as to why as well. I just tried moving the files to a fresh
branch and used git cl issue, now 3 of the files are deleted, but 2 remain (??).

>
https://codereview.chromium.org/12211049/diff/39001/chrome/browser/icon_loade...
> File chrome/browser/icon_loader_android.cc (right):
> 
>
https://codereview.chromium.org/12211049/diff/39001/chrome/browser/icon_loade...
> chrome/browser/icon_loader_android.cc:8: IconGroupID
> IconLoader::ReadGroupIDFromFilepath(
> Above static member functions, we generally put a "// static" comment. Same
> applies to all files.
> 
>
https://codereview.chromium.org/12211049/diff/39001/chrome/browser/icon_loade...
> File chrome/browser/icon_loader_chromeos.cc (right):
> 
>
https://codereview.chromium.org/12211049/diff/39001/chrome/browser/icon_loade...
> chrome/browser/icon_loader_chromeos.cc:12: #include "base/file_path.h"
> This file is now in base/files/

[Robert Sesek, 2013-03-11 15:19:10]

LGTM!

https://codereview.chromium.org/12211049/diff/25002/chrome/browser/icon_manag...
File chrome/browser/icon_manager_android.cc (right):

https://codereview.chromium.org/12211049/diff/25002/chrome/browser/icon_manag...
chrome/browser/icon_manager_android.cc:1: // Copyright (c) 2012 The Chromium
Authors. All rights reserved.
On 2013/03/08 18:41:26, rsesek wrote:
> These files aren't coming up as 'D' in the patch set. Not sure why…

I'm no expert in the mysteries of Rietveld, but I think it means something is
wrong with the "base file."

[shatch, 2013-03-15 23:23:57]

On 2013/03/11 15:19:10, rsesek wrote:
> LGTM!
> 
>
https://codereview.chromium.org/12211049/diff/25002/chrome/browser/icon_manag...
> File chrome/browser/icon_manager_android.cc (right):
> 
>
https://codereview.chromium.org/12211049/diff/25002/chrome/browser/icon_manag...
> chrome/browser/icon_manager_android.cc:1: // Copyright (c) 2012 The Chromium
> Authors. All rights reserved.
> On 2013/03/08 18:41:26, rsesek wrote:
> > These files aren't coming up as 'D' in the patch set. Not sure why…
> 
> I'm no expert in the mysteries of Rietveld, but I think it means something is
> wrong with the "base file."

I'm not sure how to fix this delete issue in Rietveld, so if it's alright, I'll
just make another issue and upload there?

[Robert Sesek, 2013-03-18 17:19:21]

If you look at the raw patch (which is what will be used by CQ) it has "deleted
file mode 100644", so I think it'll do the right thing.

[commit-bot: I haz the power, 2013-03-19 16:26:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhatch@chromium.org/12211049/81001

[commit-bot: I haz the power, 2013-03-19 16:27:02]

Presubmit check for 12211049-81001 failed and returned exit status 1.

INFO:root:Found 15 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/extensions/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/ui/views/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/ui/webui/PRESUBMIT.py

** Presubmit Warnings **
New code should not use wstrings.  If you are calling a cross-platform API that
accepts a wstring, fix the API.
    chrome/browser/icon_loader_win.cc:21

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/browser/icon_loader_win.cc
    chrome/browser/icon_loader_android.cc
    chrome/browser/icon_loader_linux.cc
    chrome/browser/ui/views/download/download_item_view.cc
    chrome/browser/icon_loader.h
    chrome/browser/icon_manager.cc
    chrome/browser/icon_manager.h
    chrome/browser/ui/webui/fileicon_source.cc
    chrome/browser/icon_loader.cc
    chrome/browser/extensions/api/downloads/downloads_api.cc
    chrome/browser/icon_loader_chromeos.cc
    chrome/chrome_browser.gypi
    chrome/browser/icon_loader_mac.mm
    chrome/browser/ui/webui/downloads_dom_handler.cc

Presubmit checks took 1.9s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[brettw, 2013-03-28 22:40:05]

owners lgtm rubberstamp.

[commit-bot: I haz the power, 2013-03-29 15:32:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhatch@chromium.org/12211049/81001

[commit-bot: I haz the power, 2013-03-29 15:32:24]

Failed to apply patch for chrome/browser/icon_loader_mac.mm:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/icon_loader_mac.mm
  Hunk #1 FAILED at 7.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/icon_loader_mac.mm.rej

Patch:       chrome/browser/icon_loader_mac.mm
Index: chrome/browser/icon_loader_mac.mm
diff --git a/chrome/browser/icon_loader_mac.mm
b/chrome/browser/icon_loader_mac.mm
index
7f522b17b75a9e5e94752ff43a6d05e19395b972..4573d1d4bf797183cc7b4e7fa5b3c29345d3b4c9
100644
--- a/chrome/browser/icon_loader_mac.mm
+++ b/chrome/browser/icon_loader_mac.mm
@@ -7,12 +7,19 @@
 #import <AppKit/AppKit.h>
 
 #include "base/bind.h"
+#include "base/files/file_path.h"
 #include "base/message_loop.h"
 #include "base/threading/thread.h"
 #include "base/sys_string_conversions.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_util_mac.h"
 
+// static
+IconGroupID IconLoader::ReadGroupIDFromFilepath(
+    const base::FilePath& filepath) {
+  return filepath.Extension();
+}
+
 void IconLoader::ReadIcon() {
   NSString* group = base::SysUTF8ToNSString(group_);
   NSWorkspace* workspace = [NSWorkspace sharedWorkspace];

[commit-bot: I haz the power, 2013-04-02 16:26:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhatch@chromium.org/12211049/92001

[commit-bot: I haz the power, 2013-04-02 16:26:38]

Presubmit check for 12211049-92001 failed and returned exit status 1.

INFO:root:Found 15 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/extensions/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/ui/views/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/ui/webui/PRESUBMIT.py

** Presubmit Warnings **
New code should not use wstrings.  If you are calling a cross-platform API that
accepts a wstring, fix the API.
    chrome/browser/icon_loader_win.cc:21

Presubmit checks took 1.9s to calculate.

[commit-bot: I haz the power, 2013-04-03 16:44:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhatch@chromium.org/12211049/99001

[commit-bot: I haz the power, 2013-04-03 20:51:01]

Change committed as 192143