mash: Replace int ShelfIDs with AppLaunchID strings.

ash/public/cpp/shelf_struct_traits.cc

 // Copyright 2017 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.
 
 #include "ash/public/cpp/shelf_struct_traits.h"
 
 #include "mojo/common/common_custom_types_struct_traits.h"
 #include "skia/public/interfaces/bitmap_skbitmap_struct_traits.h"
 #include "ui/gfx/image/image_skia.h"
 
 namespace mojo {
 
 using ShelfItemStructTraits =
     StructTraits<ash::mojom::ShelfItemDataView, ash::ShelfItem>;
 
 // static
+bool StructTraits<ash::mojom::ShelfIDDataView, ash::ShelfID>::Read(
+    ash::mojom::ShelfIDDataView data,
+    ash::ShelfID* out) {
+  std::string app_id;
+  std::string launch_id;
+  if (!data.ReadAppId(&app_id) || !data.ReadLaunchId(&launch_id))
+    return false;
+  *out = ash::ShelfID(app_id, launch_id);

[James Cook, 2017/05/04 16:38:48]

Reading a ShelfID looks like it allocates and copies strings at least twice. Is
it possible to friend the structtraits in ShelfID / AppLaunchID and then read
the data directly into out->app_id_ and out->launch_id_ ?

[msw, 2017/05/04 19:05:56]

WDYT about instead making AppLaunchId a struct with public |app_id| and
|launch_id| (in a followup or now)? It seems a bit odd that ShelfItem is such a
struct, but the simple two-string id it contains is a class with accessors.
Friending traits just adds complexity on top of unnecessary complexity.

[James Cook, 2017/05/04 19:45:07]

Yeah, I think making ShelfID / AppLaunchId a struct would be better. Do that.

If you think it will touch many of the lines that you're touching in this CL I
would do it in this one. Otherwise a followup would be fine.

I would also check here that if the launch_id is non-empty that the app_id is
non-empty too.

[msw, 2017/05/04 20:39:07]

Done.

+  return true;
+}
+
+// static
 const SkBitmap& ShelfItemStructTraits::image(const ash::ShelfItem& i) {
   // TODO(crbug.com/655874): Remove this when we have a gfx::Image[Skia] mojom.
   static const SkBitmap kNullSkBitmap;
   return i.image.isNull() ? kNullSkBitmap : *i.image.bitmap();
 }
 
 // static
 bool ShelfItemStructTraits::Read(ash::mojom::ShelfItemDataView data,
                                  ash::ShelfItem* out) {
   SkBitmap image;
-  std::string app_id;
-  std::string launch_id;
   if (!data.ReadType(&out->type) || !data.ReadImage(&image) ||
-      !data.ReadStatus(&out->status) || !data.ReadAppId(&app_id) ||
-      !data.ReadLaunchId(&launch_id) || !data.ReadTitle(&out->title)) {
+      !data.ReadStatus(&out->status) || !data.ReadShelfId(&out->id) ||
+      !data.ReadTitle(&out->title)) {
     return false;
   }
-  out->id = data.shelf_id();
-  out->app_launch_id = ash::AppLaunchId(app_id, launch_id);
   // TODO(crbug.com/655874): Support high-dpi images via gfx::Image[Skia] mojom.
   out->image = gfx::ImageSkia::CreateFrom1xBitmap(image);
   out->shows_tooltip = data.shows_tooltip();
   out->pinned_by_policy = data.pinned_by_policy();
   return true;
 }
 
 }  // namespace mojo