Add "usages" field to Image in WebApk.proto

chrome/browser/android/webapk/webapk.proto

 // 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.
 
 syntax = "proto2";
 
 option optimize_for = LITE_RUNTIME;
 
 package webapk;
 
@@ skipping 53 matching lines @@
   repeated Image icons = 6;
   optional string orientation = 9;
   optional string display_mode = 10;
   optional string theme_color = 11;
   optional string background_color = 12;
 
   reserved 3, 7, 8, 13, 14;
 }
 
 message Image {
+
+  enum Usage {

[hartmanng, 2017/02/07 14:53:22]

It could be useful to have a

USAGE_UNDEFINED = 0;

as well.

[F, 2017/02/08 15:32:14]

Resolved offline :)

+    PRIMARY_ICON = 1;

[hartmanng, 2017/02/07 14:53:22]

I haven't been following discussions around this - can you explain quickly why
we're diverging from the spec here? Under what conditions do we determine that
an icon is a "primary icon"? Is it just whenever it's not a badge?

[hartmanng, 2017/02/07 18:18:29]

We've discussed offline and I understand the situation now. Thanks for
explaining, Felix.

[F, 2017/02/08 15:32:14]

:)

+    BADGE_ICON = 2;
+  }
+
   // Image's URL.
   optional string src = 1;
 
   // Murmur2 hash of the icon's bytes. There should not be any transformations
   // applied to the icon's bytes prior to taking the Murmur2 hash.
   optional string hash = 5;
 
   // Actual bytes of the image. This image may be re-encoded from the original
   // image and may not match the murmur2 hash field above.
   optional bytes image_data = 6;
 
+  // Specifies intended usages for the image.
+  repeated Usage usages = 7;

[pkotwicz, 2017/02/07 14:38:06]

If we end up putting this information in the Android Manifest, we should make
this a repeated field of string.

The reason is that other browsers may want to extract the values from the
Android Manifest. The string values are specified by the spec so are unlikely to
change. The enum values are not specified by the spec so using them from a
non-Chrome browser perspective is more risky.

[hartmanng, 2017/02/07 14:53:22]

We've actually already added a Purpose member to the server-side proto (my
apologies for not pointing you to it earlier, completely slipped my mind).

We don't use it for anything yet, and it would be quite easy for us to change,
so we can still support the strategy in this CL if it's better. However, if we
go with "Usages" instead of "Purposes" here, please mark 7 as "reserved" and use
8 for Usages to avoid proto incompatibility.

[hartmanng, 2017/02/07 18:18:29]

To sum up our offline conversation, I think what we should do (for now, at
least), is that you should go forward with this CL and land a Usages member, and
leave Purposes out for the time being.

Please do mark 7 as reserved and move Usages to 8 in this CL, though.

[hartmanng, 2017/02/07 18:18:29]

That's not entirely true. The Usage type isn't exactly the same as the spec's
official Purpose type. It's true that the spec defines "badge" as a kind of
Purpose, but as far as I understand, the spec has no concept of a primary icon -
that's a Chrome abstraction.

[F, 2017/02/08 15:32:14]

Done. Thanks for pointing out 7 is already used :)

+
   reserved 2 to 4;
 }