[Suggestions] Make ThumbnailManager implement ImageManager

[Suggestions] Make ThumbnailManager implement ImageManager

In a componentized world, ThumbnailManager will stay in Chrome and ImageManager will be the interface on which SuggestionsService will depend on.

BUG=387751
TBR=jam
TEST=Suggestions*,ThumbnailManager*

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

[Mathieu, 2014-07-16 20:39:09]

Hello Samuel, 

Want to have a look?

[huangs, 2014-07-16 22:22:24]

First round.

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
File chrome/browser/search/suggestions/image_manager.h (right):

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/image_manager.h:1: // Copyright 2014 The
Chromium Authors. All rights reserved.
Should this file live outside Chrome?

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/image_manager.h:23: virtual void
InitializeImageMap(const SuggestionsProfile& suggestions) = 0;
Should the concept of "image map" be a generic interface of ImageManager, or
implementation detail of subclasses?  If former, maybe move |thumbnail_url_map_|
to this class, along with routines that use them (e.g., GetBitmapFromCache()) to
a .cc file.  If latter, them we can remove this and the dependence on
suggestions.pb.h.

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/image_manager.h:26: // |callback| with Bitmap
pointer if found, and NULL otherwise. Should be
NIT: unwrap |callback| to previous line.

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/image_manager.h:28: virtual void
GetImageForURL(
Would "UI thread" be a Chrome (even browser) concept, or is it okay to have this
in components?

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/image_manager.h:31: };
DISALLOW_COPY_AND_ASSIGN ?

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
File chrome/browser/search/suggestions/suggestions_service.h (right):

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/suggestions_service.h:166:
scoped_ptr<ImageManager> thumbnail_manager_;
*If* you're keeping the name "ImageManager", maybe rename variable to match?

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
File chrome/browser/search/suggestions/suggestions_service_unittest.cc (right):

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/suggestions_service_unittest.cc:109:
MOCK_METHOD1(InitializeImageMap, void(const SuggestionsProfile&));
Note that InitializeImageMap() is never used.

[Mathieu, 2014-07-17 16:06:36]

See my replies, thanks!

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
File chrome/browser/search/suggestions/image_manager.h (right):

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/image_manager.h:1: // Copyright 2014 The
Chromium Authors. All rights reserved.
On 2014/07/16 22:22:23, huangs1 wrote:
> Should this file live outside Chrome?

Yes it will move inside a component soon, next CL.

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/image_manager.h:23: virtual void
InitializeImageMap(const SuggestionsProfile& suggestions) = 0;
On 2014/07/16 22:22:24, huangs1 wrote:
> Should the concept of "image map" be a generic interface of ImageManager, or
> implementation detail of subclasses?  If former, maybe move
|thumbnail_url_map_|
> to this class, along with routines that use them (e.g., GetBitmapFromCache())
to
> a .cc file.  If latter, them we can remove this and the dependence on
> suggestions.pb.h.

Discussed offline.

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/image_manager.h:26: // |callback| with Bitmap
pointer if found, and NULL otherwise. Should be
On 2014/07/16 22:22:23, huangs1 wrote:
> NIT: unwrap |callback| to previous line.

Done.

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/image_manager.h:28: virtual void
GetImageForURL(
On 2014/07/16 22:22:24, huangs1 wrote:
> Would "UI thread" be a Chrome (even browser) concept, or is it okay to have
this
> in components?

Done.

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/image_manager.h:31: };
On 2014/07/16 22:22:23, huangs1 wrote:
> DISALLOW_COPY_AND_ASSIGN ?

Done.

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
File chrome/browser/search/suggestions/suggestions_service.h (right):

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/suggestions_service.h:166:
scoped_ptr<ImageManager> thumbnail_manager_;
On 2014/07/16 22:22:24, huangs1 wrote:
> *If* you're keeping the name "ImageManager", maybe rename variable to match?

Acknowledged.

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
File chrome/browser/search/suggestions/suggestions_service_unittest.cc (right):

https://codereview.chromium.org/392983007/diff/1/chrome/browser/search/sugges...
chrome/browser/search/suggestions/suggestions_service_unittest.cc:109:
MOCK_METHOD1(InitializeImageMap, void(const SuggestionsProfile&));
On 2014/07/16 22:22:24, huangs1 wrote:
> Note that InitializeImageMap() is never used.

Changed to Initialize per discussion

[huangs, 2014-07-17 18:23:35]

LGTM with nits.

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
File chrome/browser/search/suggestions/image_manager.h (right):

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/image_manager.h:10: #include
"chrome/browser/search/suggestions/proto/suggestions.pb.h"
Note that SuggestionsProfile can be pre-declared (don't have to).

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/image_manager.h:22: // Initializes the
internal data representation with the data received from
NIT:
// (Re)Initializes states using data received from a SuggestionService. We're
not doing this in the constructor because an instance may be long-lived.

(or something)

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
File chrome/browser/search/suggestions/suggestions_service_factory.cc (right):

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/suggestions_service_factory.cc:5: #include
"chrome/browser/search/suggestions/suggestions_service_factory.h"
The .h file should include "base/memory/scoped_ptr.h"

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
File chrome/browser/search/suggestions/thumbnail_manager.cc (right):

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/thumbnail_manager.cc:71: // |database_| can be
null if something went wrong in initialization.
NIT: NULL

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
File chrome/browser/search/suggestions/thumbnail_manager.h (right):

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/thumbnail_manager.h:46: // Initializes the
|thumbnail_url_map_| with the proper mapping from website
Add something similar to
// ImageManager interface.
Update comment to match image_manager.h's, or simply remove it.

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/thumbnail_manager.h:53: virtual void
GetImageForURL(
// ImageManager interface.

The UI thread constraint departs from the base classes.  If it's a property of
this class, then comment as such.  Otherwise leave things the way they are.

[Mathieu, 2014-07-17 18:29:13]

Thanks for the comments :)

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
File chrome/browser/search/suggestions/image_manager.h (right):

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/image_manager.h:10: #include
"chrome/browser/search/suggestions/proto/suggestions.pb.h"
On 2014/07/17 18:23:35, huangs1 wrote:
> Note that SuggestionsProfile can be pre-declared (don't have to).

Since there's no CC I prefer to have it here

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/image_manager.h:22: // Initializes the
internal data representation with the data received from
On 2014/07/17 18:23:35, huangs1 wrote:
> NIT:
> // (Re)Initializes states using data received from a SuggestionService. We're
> not doing this in the constructor because an instance may be long-lived.
> 
> (or something)

Done.

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/image_manager.h:22: // Initializes the
internal data representation with the data received from
On 2014/07/17 18:23:35, huangs1 wrote:
> NIT:
> // (Re)Initializes states using data received from a SuggestionService. We're
> not doing this in the constructor because an instance may be long-lived.
> 
> (or something)

Done.

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
File chrome/browser/search/suggestions/suggestions_service_factory.cc (right):

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/suggestions_service_factory.cc:5: #include
"chrome/browser/search/suggestions/suggestions_service_factory.h"
On 2014/07/17 18:23:35, huangs1 wrote:
> The .h file should include "base/memory/scoped_ptr.h"

Acknowledged.

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
File chrome/browser/search/suggestions/thumbnail_manager.cc (right):

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/thumbnail_manager.cc:71: // |database_| can be
null if something went wrong in initialization.
On 2014/07/17 18:23:35, huangs1 wrote:
> NIT: NULL

Done.

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
File chrome/browser/search/suggestions/thumbnail_manager.h (right):

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/thumbnail_manager.h:46: // Initializes the
|thumbnail_url_map_| with the proper mapping from website
On 2014/07/17 18:23:35, huangs1 wrote:
> Add something similar to
> // ImageManager interface.
> Update comment to match image_manager.h's, or simply remove it.

Done.

https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
chrome/browser/search/suggestions/thumbnail_manager.h:53: virtual void
GetImageForURL(
On 2014/07/17 18:23:35, huangs1 wrote:
> // ImageManager interface.
> 
> The UI thread constraint departs from the base classes.  If it's a property of
> this class, then comment as such.  Otherwise leave things the way they are.

Done.

[Mathieu, 2014-07-17 18:32:36]

On 2014/07/17 18:29:13, Mathieu Perreault wrote:
> Thanks for the comments :)
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> File chrome/browser/search/suggestions/image_manager.h (right):
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> chrome/browser/search/suggestions/image_manager.h:10: #include
> "chrome/browser/search/suggestions/proto/suggestions.pb.h"
> On 2014/07/17 18:23:35, huangs1 wrote:
> > Note that SuggestionsProfile can be pre-declared (don't have to).
> 
> Since there's no CC I prefer to have it here
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> chrome/browser/search/suggestions/image_manager.h:22: // Initializes the
> internal data representation with the data received from
> On 2014/07/17 18:23:35, huangs1 wrote:
> > NIT:
> > // (Re)Initializes states using data received from a SuggestionService.
We're
> > not doing this in the constructor because an instance may be long-lived.
> > 
> > (or something)
> 
> Done.
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> chrome/browser/search/suggestions/image_manager.h:22: // Initializes the
> internal data representation with the data received from
> On 2014/07/17 18:23:35, huangs1 wrote:
> > NIT:
> > // (Re)Initializes states using data received from a SuggestionService.
We're
> > not doing this in the constructor because an instance may be long-lived.
> > 
> > (or something)
> 
> Done.
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> File chrome/browser/search/suggestions/suggestions_service_factory.cc (right):
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> chrome/browser/search/suggestions/suggestions_service_factory.cc:5: #include
> "chrome/browser/search/suggestions/suggestions_service_factory.h"
> On 2014/07/17 18:23:35, huangs1 wrote:
> > The .h file should include "base/memory/scoped_ptr.h"
> 
> Acknowledged.
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> File chrome/browser/search/suggestions/thumbnail_manager.cc (right):
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> chrome/browser/search/suggestions/thumbnail_manager.cc:71: // |database_| can
be
> null if something went wrong in initialization.
> On 2014/07/17 18:23:35, huangs1 wrote:
> > NIT: NULL
> 
> Done.
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> File chrome/browser/search/suggestions/thumbnail_manager.h (right):
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> chrome/browser/search/suggestions/thumbnail_manager.h:46: // Initializes the
> |thumbnail_url_map_| with the proper mapping from website
> On 2014/07/17 18:23:35, huangs1 wrote:
> > Add something similar to
> > // ImageManager interface.
> > Update comment to match image_manager.h's, or simply remove it.
> 
> Done.
> 
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> chrome/browser/search/suggestions/thumbnail_manager.h:53: virtual void
> GetImageForURL(
> On 2014/07/17 18:23:35, huangs1 wrote:
> > // ImageManager interface.
> > 
> > The UI thread constraint departs from the base classes.  If it's a property
of
> > this class, then comment as such.  Otherwise leave things the way they are.
> 
> Done.

TBR jam for chrome/chrome_browser.gypi (new file)

[Mathieu, 2014-07-17 18:32:42]

The CQ bit was checked by mathp@chromium.org

[commit-bot: I haz the power, 2014-07-17 18:33:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mathp@chromium.org/392983007/40001

[jam, 2014-07-17 20:25:45]

On 2014/07/17 18:32:36, Mathieu Perreault wrote:
> On 2014/07/17 18:29:13, Mathieu Perreault wrote:
> > Thanks for the comments :)
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > File chrome/browser/search/suggestions/image_manager.h (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > chrome/browser/search/suggestions/image_manager.h:10: #include
> > "chrome/browser/search/suggestions/proto/suggestions.pb.h"
> > On 2014/07/17 18:23:35, huangs1 wrote:
> > > Note that SuggestionsProfile can be pre-declared (don't have to).
> > 
> > Since there's no CC I prefer to have it here
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > chrome/browser/search/suggestions/image_manager.h:22: // Initializes the
> > internal data representation with the data received from
> > On 2014/07/17 18:23:35, huangs1 wrote:
> > > NIT:
> > > // (Re)Initializes states using data received from a SuggestionService.
> We're
> > > not doing this in the constructor because an instance may be long-lived.
> > > 
> > > (or something)
> > 
> > Done.
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > chrome/browser/search/suggestions/image_manager.h:22: // Initializes the
> > internal data representation with the data received from
> > On 2014/07/17 18:23:35, huangs1 wrote:
> > > NIT:
> > > // (Re)Initializes states using data received from a SuggestionService.
> We're
> > > not doing this in the constructor because an instance may be long-lived.
> > > 
> > > (or something)
> > 
> > Done.
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > File chrome/browser/search/suggestions/suggestions_service_factory.cc
(right):
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > chrome/browser/search/suggestions/suggestions_service_factory.cc:5: #include
> > "chrome/browser/search/suggestions/suggestions_service_factory.h"
> > On 2014/07/17 18:23:35, huangs1 wrote:
> > > The .h file should include "base/memory/scoped_ptr.h"
> > 
> > Acknowledged.
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > File chrome/browser/search/suggestions/thumbnail_manager.cc (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > chrome/browser/search/suggestions/thumbnail_manager.cc:71: // |database_|
can
> be
> > null if something went wrong in initialization.
> > On 2014/07/17 18:23:35, huangs1 wrote:
> > > NIT: NULL
> > 
> > Done.
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > File chrome/browser/search/suggestions/thumbnail_manager.h (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > chrome/browser/search/suggestions/thumbnail_manager.h:46: // Initializes the
> > |thumbnail_url_map_| with the proper mapping from website
> > On 2014/07/17 18:23:35, huangs1 wrote:
> > > Add something similar to
> > > // ImageManager interface.
> > > Update comment to match image_manager.h's, or simply remove it.
> > 
> > Done.
> > 
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > chrome/browser/search/suggestions/thumbnail_manager.h:53: virtual void
> > GetImageForURL(
> > On 2014/07/17 18:23:35, huangs1 wrote:
> > > // ImageManager interface.
> > > 
> > > The UI thread constraint departs from the base classes.  If it's a
property
> of
> > > this class, then comment as such.  Otherwise leave things the way they
are.
> > 
> > Done.
> 
> TBR jam for chrome/chrome_browser.gypi (new file)

chrome/OWNERS has * for gypis

[Mathieu, 2014-07-17 20:31:42]

On 2014/07/17 20:25:45, jam wrote:
> On 2014/07/17 18:32:36, Mathieu Perreault wrote:
> > On 2014/07/17 18:29:13, Mathieu Perreault wrote:
> > > Thanks for the comments :)
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > File chrome/browser/search/suggestions/image_manager.h (right):
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > chrome/browser/search/suggestions/image_manager.h:10: #include
> > > "chrome/browser/search/suggestions/proto/suggestions.pb.h"
> > > On 2014/07/17 18:23:35, huangs1 wrote:
> > > > Note that SuggestionsProfile can be pre-declared (don't have to).
> > > 
> > > Since there's no CC I prefer to have it here
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > chrome/browser/search/suggestions/image_manager.h:22: // Initializes the
> > > internal data representation with the data received from
> > > On 2014/07/17 18:23:35, huangs1 wrote:
> > > > NIT:
> > > > // (Re)Initializes states using data received from a SuggestionService.
> > We're
> > > > not doing this in the constructor because an instance may be long-lived.
> > > > 
> > > > (or something)
> > > 
> > > Done.
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > chrome/browser/search/suggestions/image_manager.h:22: // Initializes the
> > > internal data representation with the data received from
> > > On 2014/07/17 18:23:35, huangs1 wrote:
> > > > NIT:
> > > > // (Re)Initializes states using data received from a SuggestionService.
> > We're
> > > > not doing this in the constructor because an instance may be long-lived.
> > > > 
> > > > (or something)
> > > 
> > > Done.
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > File chrome/browser/search/suggestions/suggestions_service_factory.cc
> (right):
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > chrome/browser/search/suggestions/suggestions_service_factory.cc:5:
#include
> > > "chrome/browser/search/suggestions/suggestions_service_factory.h"
> > > On 2014/07/17 18:23:35, huangs1 wrote:
> > > > The .h file should include "base/memory/scoped_ptr.h"
> > > 
> > > Acknowledged.
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > File chrome/browser/search/suggestions/thumbnail_manager.cc (right):
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > chrome/browser/search/suggestions/thumbnail_manager.cc:71: // |database_|
> can
> > be
> > > null if something went wrong in initialization.
> > > On 2014/07/17 18:23:35, huangs1 wrote:
> > > > NIT: NULL
> > > 
> > > Done.
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > File chrome/browser/search/suggestions/thumbnail_manager.h (right):
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > chrome/browser/search/suggestions/thumbnail_manager.h:46: // Initializes
the
> > > |thumbnail_url_map_| with the proper mapping from website
> > > On 2014/07/17 18:23:35, huangs1 wrote:
> > > > Add something similar to
> > > > // ImageManager interface.
> > > > Update comment to match image_manager.h's, or simply remove it.
> > > 
> > > Done.
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/392983007/diff/20001/chrome/browser/se...
> > > chrome/browser/search/suggestions/thumbnail_manager.h:53: virtual void
> > > GetImageForURL(
> > > On 2014/07/17 18:23:35, huangs1 wrote:
> > > > // ImageManager interface.
> > > > 
> > > > The UI thread constraint departs from the base classes.  If it's a
> property
> > of
> > > > this class, then comment as such.  Otherwise leave things the way they
> are.
> > > 
> > > Done.
> > 
> > TBR jam for chrome/chrome_browser.gypi (new file)
> 
> chrome/OWNERS has * for gypis

Missed that somehow, thanks!

[commit-bot: I haz the power, 2014-07-17 23:23:44]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-07-17 23:42:15]

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

[commit-bot: I haz the power, 2014-07-17 23:42:17]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[Mathieu, 2014-07-18 00:02:49]

The CQ bit was checked by mathp@chromium.org

[commit-bot: I haz the power, 2014-07-18 00:07:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mathp@chromium.org/392983007/40001

[commit-bot: I haz the power, 2014-07-18 00:30:12]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-07-18 00:55:52]

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

[commit-bot: I haz the power, 2014-07-18 00:55:53]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[Mathieu, 2014-07-18 13:21:05]

The CQ bit was checked by mathp@chromium.org

[commit-bot: I haz the power, 2014-07-18 13:22:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mathp@chromium.org/392983007/40001

[commit-bot: I haz the power, 2014-07-18 13:23:31]

Change committed as 284077