Loading bitmap to a menu item in a loop with Picasso

Question

The code below is called in a loop. I've read this SO answer but, since I cannot setTag to a MenuItem, Target gets garbage collected. onBitmapLoaded is not called. How can I solve this issue.

The other question is, at the first launch of the app it doesn't work. How does it work after I call this method again.

private void addServiceToMenu(Service service, final MenuItem menuItem) {
    if (!TextUtils.isEmpty(service.getIconURL())) {
        Resources resources = getResources();
        final int targetWidth = resources.getDimensionPixelSize(R.dimen.menu_icon_size);
        final int targetHeight = resources.getDimensionPixelSize(R.dimen.menu_icon_size);

        final Target target = new Target() {
            @Override
            public void onBitmapLoaded(Bitmap bitmap, Picasso.LoadedFrom from) {
                Drawable drawable = new BitmapDrawable(getResources(), bitmap);

                drawable.setBounds(0, 0, targetWidth, targetHeight);
                menuItem.setIcon(drawable);
            }

            @Override
            public void onBitmapFailed(Drawable errorDrawable) { }

            @Override
            public void onPrepareLoad(Drawable placeHolderDrawable) { }
        };

        Picasso.with(MainActivity.this).load(service.getIconURL())
                .resize(targetWidth, targetHeight)
                .into(target);
    }
}

Surely there's somewhere reasonable to hold the `Target` reference, though. As a field on the object that has this method? Some Activity, controller, or View? — Eric Cochran, May 11 '16 at 01:36
As for the other question, the reason the callback doesn't come back the first time is that the `Target` reference is lost while loading, but, the second time, we have the response cached and we callback immediately to the `Target`. — Eric Cochran, May 11 '16 at 01:37

Jared Rummler · Answer 1 · 2016-05-11T02:04:17.387

Create a class where you can keep a strong reference to a Target.

Full working example:

public class MainActivity extends AppCompatActivity {

  private final List<Service> services = new ArrayList<>();

  {
    // add arbitrary data just for the example
    services.add(new Service("Android",
        "https://github.com/google/material-design-icons/raw/master/action/drawable-xhdpi/ic_android_black_24dp.png"));
    services.add(new Service("Account",
        "https://github.com/google/material-design-icons/raw/master/action/drawable-xhdpi/ic_account_circle_black_24dp.png"));
    services.add(new Service("Shopping",
        "https://github.com/google/material-design-icons/raw/master/action/drawable-xhdpi/ic_add_shopping_cart_black_24dp.png"));
  }

  @Override public boolean onCreateOptionsMenu(Menu menu) {
    for (Service service : services) {
      // create a few MenuItems. Normally done in XML.
      MenuItem menuItem = menu.add(service.getName());
      menuItem.setShowAsAction(MenuItem.SHOW_AS_ACTION_ALWAYS);
      // load the icon using Picasso
      addServiceToMenu(service, menuItem);
    }
    return super.onCreateOptionsMenu(menu);
  }

  private void addServiceToMenu(Service service, final MenuItem menuItem) {
    if (!TextUtils.isEmpty(service.getIconURL())) {
      Resources resources = getResources();
      final int targetWidth = resources.getDimensionPixelSize(R.dimen.menu_icon_size);
      final int targetHeight = resources.getDimensionPixelSize(R.dimen.menu_icon_size);
      final MenuItemIconLoader loader = new MenuItemIconLoader(menuItem, targetHeight, targetWidth);
      loader.load(MainActivity.this, service);
    }
  }

  class MenuItemIconLoader {

    private final WeakReference<MenuItem> itemWeakReference;
    private final int targetHeight;
    private final int targetWidth;

    public MenuItemIconLoader(MenuItem menuItem, int targetHeight, int targetWidth) {
      this.itemWeakReference = new WeakReference<>(menuItem);
      this.targetHeight = targetHeight;
      this.targetWidth = targetWidth;
    }

    private final Target target = new Target() {

      @Override public void onBitmapLoaded(Bitmap bitmap, Picasso.LoadedFrom from) {
        MenuItem menuItem = itemWeakReference.get();
        if (menuItem != null) {
          Drawable drawable = new BitmapDrawable(getResources(), bitmap);
          drawable.setBounds(0, 0, targetWidth, targetHeight);
          menuItem.setIcon(drawable);
        }
      }

      @Override public void onBitmapFailed(Drawable errorDrawable) {

      }

      @Override public void onPrepareLoad(Drawable placeHolderDrawable) {

      }
    };

    public void load(Context context, Service service) {
      Picasso.with(context).load(service.getIconURL()).resize(targetWidth, targetHeight).into(target);
    }

  }

  static class Service {

    private String name;
    private String iconUrl;

    public Service(String name, String iconUrl) {
      this.name = name;
      this.iconUrl = iconUrl;
    }

    public String getName() {
      return name;
    }

    public String getIconURL() {
      return iconUrl;
    }

  }

}

Your target is being garbage collected because it is a local variable declared inside a method. Picasso loads the drawable on another thread and your local variable is garbage collected. Making the target a instance variable will keep it in memory. https://github.com/square/picasso/issues/83 — Jared Rummler, May 11 '16 at 10:00
`MenuItemIconLoader ` is also a local variable declared inside the same method. What keeps it in memory? Why isn't it garbage collected? Also, why did you use `WeakReference` ? — osrl, May 11 '16 at 10:16
The `Target` is a instance variable and that is what keeps it in memory. I used a `WeakReference` for the `MenuItem` because I'm not sure if it holds onto the `activity` and could potentially cause a memory leak. You may not need it. — Jared Rummler, May 11 '16 at 11:36
I mean what keeps `MenuItemIconLoader ` in memory. It's not instance variable either. It is also a local variable. — osrl, May 11 '16 at 11:53
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/111639/discussion-between-osrl-and-jared-rummler). — osrl, May 11 '16 at 12:45
I've finally found time to implement this. But it didn't work. — osrl, Jun 02 '16 at 12:40

score 1 · Answer 2 · edited May 23 '17 at 10:29

I've implemented @Jared's answer but it didn't work. So I've decided to keep all MenuItemIconLoaders in a list and it worked. I think I was right about the question:

What keeps 'MenuItemIconLoader' in memory?

Here is my solution:

List<Service> mServices = new ArrayList<>();
//To keep them in memory
final List<MenuItemIconLoader> mIconLoaderList = new ArrayList<>();
....
//
for (int i = 0; i < mServices.size(); i++) {
    Service service = mServices.get(i);
    final MenuItem menuItem = menu.add(SERVICES, Menu.NONE + i, i + 1, service.getTitle())
            .setCheckable(true);
    mIconLoaderList.add(new MenuItemIconLoader(MainActivity.this, menuItem));
}
addServicesToMenu();

....
private void addServicesToMenu() {
    for (int i = 0; i < mServices.size(); i++) {
        Service service = mServices.get(i);
        MenuItemIconLoader iconLoader = mIconLoaderList.get(i);

        if (!TextUtils.isEmpty(service.getIconURL())) {
            iconLoader.load(service);
        }
    }
}

Loading bitmap to a menu item in a loop with Picasso

2 Answers2