Implement an ObjC wrapper for fetching account chooser avatar images.

chrome/browser/ui/cocoa/passwords/account_avatar_fetcher_manager.mm

Index: chrome/browser/ui/cocoa/passwords/account_avatar_fetcher_manager.mm
diff --git a/chrome/browser/ui/cocoa/passwords/account_avatar_fetcher_manager.mm b/chrome/browser/ui/cocoa/passwords/account_avatar_fetcher_manager.mm
new file mode 100644
index 0000000000000000000000000000000000000000..618b78220bff65b999e7f6b5fef1789ef6fa91be
--- /dev/null
+++ b/chrome/browser/ui/cocoa/passwords/account_avatar_fetcher_manager.mm
@@ -0,0 +1,85 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "chrome/browser/ui/cocoa/passwords/account_avatar_fetcher_manager.h"
+
+#include "base/memory/weak_ptr.h"
+#import "chrome/browser/ui/cocoa/passwords/credential_item_view.h"
+#include "chrome/browser/ui/passwords/account_avatar_fetcher.h"
+#include "chrome/browser/ui/passwords/manage_passwords_view_utils.h"
+#include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/image/image_skia_util_mac.h"
+
+class AccountAvatarFetcherBridge;
+
+@interface AccountAvatarFetcherManager()
+- (void)updateAvatar:(NSImage*)image
+          fromBridge:(AccountAvatarFetcherBridge*)bridge
+             forView:(CredentialItemView*)view;
+@end
+
+class AccountAvatarFetcherBridge
+    : public AccountAvatarFetcherDelegate,
+      public base::SupportsWeakPtr<AccountAvatarFetcherBridge> {
+ public:
+  AccountAvatarFetcherBridge(AccountAvatarFetcherManager* manager,
+                             CredentialItemView* view);
+  virtual ~AccountAvatarFetcherBridge();
+
+  // AccountAvatarFetcherDelegate:
+  void UpdateAvatar(const gfx::ImageSkia& image) override;
+
+ private:
+  AccountAvatarFetcherManager* manager_;
+  CredentialItemView* view_;
+};
+
+AccountAvatarFetcherBridge::AccountAvatarFetcherBridge(
+    AccountAvatarFetcherManager* manager,
+    CredentialItemView* view)
+    : manager_(manager), view_(view) {
+}
+
+AccountAvatarFetcherBridge::~AccountAvatarFetcherBridge() = default;
+
+void AccountAvatarFetcherBridge::UpdateAvatar(const gfx::ImageSkia& image) {
+  [manager_
+      updateAvatar:gfx::NSImageFromImageSkia(ScaleImageForAccountAvatar(image))
+        fromBridge:this
+           forView:view_];
+}
+
+@implementation AccountAvatarFetcherManager
+
+- (id)initWithRequestContext:
+        (scoped_refptr<net::URLRequestContextGetter>)requestContext {
+  if ((self = [super init])) {
+    requestContext_ = requestContext;
+  }
+  return self;
+}
+
+- (void)startRequestWithFetcher:(AccountAvatarFetcher*)fetcher {
+  fetcher->Start(requestContext_.get());
+}
+
+- (void)fetchAvatar:(const GURL&)avatarURL forView:(CredentialItemView*)view {
+  scoped_ptr<AccountAvatarFetcherBridge> bridge(
+      new AccountAvatarFetcherBridge(self, view));
+  AccountAvatarFetcher* fetcher =
+      new AccountAvatarFetcher(avatarURL, bridge->AsWeakPtr());
+  bridges_.push_back(bridge.Pass());
+  [self startRequestWithFetcher:fetcher];
+}
+
+- (void)updateAvatar:(NSImage*)image
+          fromBridge:(AccountAvatarFetcherBridge*)bridge
+             forView:(CredentialItemView*)view {
+  [view updateAvatar:image];
+  auto it = std::find(bridges_.begin(), bridges_.end(), bridge);

[dconnelly, 2015/02/19 10:37:12]

It turns out the std::erase(std::remove(bridges_.begin(), bridges_.end(),
bridge), bridges_.end()) can sometimes stomp memory when applied to a
ScopedVector.

std::remove doesn't guarantee that the elements from the new end iterator to the
old end iterator are actually removed elements -- in fact, the elements in that
range are undefined.

http://www.cplusplus.com/reference/algorithm/remove/

So in some cases, after calling std::remove, all the elements in the range
[bridges_.begin(), bridges_.end()) were the same pointer, and since this is a
ScopedVector, calling bridges_.erase() on any of them led to a double-free.

Given this, it doesn't seem like std::remove is safe to use on ScopedVectors.
I've changed the code back.

[groby-ooo-7-16, 2015/02/19 17:07:20]

Sigh. Bitten by both scopedness and STL annoyingness.

My apologies for misleading you.

+  if (it != bridges_.end())
+    bridges_.erase(it);
+}
+
+@end