Extract AppBannerDataFetcher into an InstallableManager.

chrome/browser/installable/installable_property.h

+// Copyright 2016 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.
+
+#ifndef CHROME_BROWSER_INSTALLABLE_INSTALLABLE_PROPERTY_H_
+#define CHROME_BROWSER_INSTALLABLE_INSTALLABLE_PROPERTY_H_
+
+#include <memory>
+
+#include "chrome/browser/installable/installable_logging.h"
+#include "content/public/common/manifest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "url/gurl.h"
+
+// Resource and status structs used by the InstallableManager to internally keep
+// track of what has and hasn't been fetched, as well as the errors encountered.
+struct ManifestProperty {

[benwells, 2016/07/29 04:36:55]

Any reason to move this into its own file? Seems like they're just an
implementation detail of InstallableManager.

[dominickn, 2016/07/31 23:32:05]

I was trying to keep the size of the manager file down, but I guess it's shrunk
again.

+  ManifestProperty();

[benwells, 2016/07/29 04:36:54]

Can you just have default values for everything, instead of a constructor?

[dominickn, 2016/07/31 23:32:05]

Done. The destructor is forced by chromium-style.

+  void Reset();

[benwells, 2016/07/29 04:36:54]

You could get rid of Reset on these by using unique_ptrs on the Manager to them,
and just resetting that.

[dominickn, 2016/07/31 23:32:05]

Done.

+
+  InstallableErrorCode error;
+  GURL url;
+  content::Manifest manifest;
+  bool fetched;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ManifestProperty);
+};
+
+struct BooleanProperty {
+  BooleanProperty();
+  void Reset();
+
+  InstallableErrorCode error;
+  bool value;
+  bool fetched;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BooleanProperty);
+};
+
+struct IconProperty {
+  IconProperty();
+  IconProperty(IconProperty&& other);
+  ~IconProperty();
+  IconProperty& operator=(IconProperty&& other);
+
+  InstallableErrorCode error;
+  GURL url;
+  std::unique_ptr<SkBitmap> icon;
+  bool fetched;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(IconProperty);

[benwells, 2016/07/29 04:36:54]

Why have disallow copy and assign on this? Seems like you have manually added
them back.

Also doesn't seem much reason to make the other two Property classes non-copy /
assignable.

[dominickn, 2016/07/31 23:32:05]

IconProperty has a std::unique_ptr, which is non-copyable, so the struct also
should be explicitly non-copyable. Since it's stored in a std::map, I also had
to define all of the constructors to make sure it would be move-only and that
the map wouldn't call the copy constructor.

+};
+
+#endif  // CHROME_BROWSER_INSTALLABLE_INSTALLABLE_PROPERTY_H_