At least the algorithm part and the interface don't seem to have anything related to Places and should be shared elsewhere. Especially cause Android has its own favicons implementation and does NOT use Places. So this interface is unreachable on Android or B2G. We will use it in Places, and whatever glue will be needed is welcome, but the whole stuff should not really live here.

I think dead metro code is the only in-tree consumer of this module. We should evaluate dropping it altogether.

The bug this blocks is still wanted though and since the color would be stored in Places that's why I suggested putting this code in Places in the first place. I think this bug should be WONTFIX/INVALID since I think it makes sense with the favicon code since mozIColorAnalyzer is made for favicons and doesn't support arbitrarily large images.

First of all, Places is about history, bookmarks and *tightly* related UI components (e.g. autocomplete). Even favicons are on the border line, and are stored in places mostly due to historical issues (and, to some extent, performance considerations). The fact that this code expects small images doesn't mean it'd be used only for favicons. I could certainly think of other use cases. Moreover, it's especially important to keep the places module focused given the limited HR dedicated to it (Needless to mention neither me nor Marco pretend to own this code in any meaningful way). The reason all of this hasn't been an issue so far is that this code is unused. Secondly, the code is both outdated and unmaintained (it'd be a jsm today, for instance) and doesn't integrate with public Places APIs the usual way (I'd expect the module to be private/semi-private and have the public API exposed in PlacesUtils). The feature requested in bug 779027, if it ever sees light (I have my doubts), doesn't require having the "raw" API in places. It could very well be in the same place with Task.jsm. Meanwhile, however, we should avoid exposing APIs that are unused. Consider this: if there are addons out there using this API by the time we get to fix bug 779027, we'd be stuck with an xpcom, callback-based API for no good reason. All in all, I think we should drop the component for now. We can bring it back later on. Should it say, however, it should be moved to toolkit/modules in the form of a module.

(In reply to Mano from comment #3) > All in all, I think we should drop the component for now. We can bring it > back later on. Should it say, however, it should be moved to toolkit/modules > in the form of a module. This seems reasonable.

Note that it is used by two extensions on AMO.

Also, is it fine to break the metro code in tree?

(In reply to Matthew N. [:MattN] (behind on reviews) from comment #6) > Also, is it fine to break the metro code in tree? Absolutely.

FYI: Bug 1008563 would like to use the component too.

fine, as long as it's moved to toolkit as a jsm module, the longer we wait the worse the situation becomes

So what about we start by removing its entry from the manifest and disabling its test, until this bug happens? This would give us a bit more time.