third_party/WebKit/public/platform/modules/installedapp/related_application.mojom - Issue 2671683002: getInstalledRelatedApps: Add browser-side Mojo service (stub).

Side by Side Diff: third_party/WebKit/public/platform/modules/installedapp/related_application.mojom

Issue 2671683002: getInstalledRelatedApps: Add browser-side Mojo service (stub). (Closed)

Patch Set: Respond to comments + increase test coverage. Created 3 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« third_party/WebKit/Source/modules/installedapp/InstalledAppController.h ('K') | « third_party/WebKit/public/platform/modules/installedapp/installed_app_provider.mojom ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 // Copyright 2017 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 module blink.mojom;

	6

	7 // Represents a system application related to a particular web app.

	8 // See: https://www.w3.org/TR/appmanifest/#related_applications-member
	dcheng 2017/03/13 07:15:08 https://www.w3.org/TR/appmanifest/#dfn-application https://www.w3.org/TR/appmanifest/#dfn-application-object might be a more helpful link (or in addition... I read the link here and had to click through once or twice to find the application object definition, which I found more useful) Matt Giuca 2017/03/14 06:38:11 Better! Thanks. Show quoted text On 2017/03/13 07:15:08, dcheng wrote: > https://www.w3.org/TR/appmanifest/#dfn-application-object might be a more > helpful link (or in addition... I read the link here and had to click through > once or twice to find the application object definition, which I found more > useful) Better! Thanks.
	9 struct RelatedApplication {

	10 string platform;

	11 string? url;
	dcheng 2017/03/13 06:16:51 Nit: please make this a mojo.common.mojom.Url. Nit: please make this a mojo.common.mojom.Url. Matt Giuca 2017/03/14 06:38:11 OK, you're right, this should be a Url. But this i Show quoted text On 2017/03/13 06:16:51, dcheng wrote: > Nit: please make this a mojo.common.mojom.Url. OK, you're right, this should be a Url. But this is a pretty big refactor which requires also converting the existing WebRelatedApplication's url field from WebString to WebUrl (or else the URL gets re-parsed several times). I did this change: https://codereview.chromium.org/2748023002 It touches too many unrelated things to do it in this CL, and also it broke my Android implementation CL in a possibly-critical way ("class file for org.chromium.url.mojom.Url not found" -- this might indicate a problem with Mojo). I'd rather save this change for a follow-up and not try to do it now. dcheng 2017/03/14 06:41:53 Please add a TODO Show quoted text On 2017/03/14 06:38:11, Matt Giuca wrote: > On 2017/03/13 06:16:51, dcheng wrote: > > Nit: please make this a mojo.common.mojom.Url. > > OK, you're right, this should be a Url. But this is a pretty big refactor which > requires also converting the existing WebRelatedApplication's url field from > WebString to WebUrl (or else the URL gets re-parsed several times). > > I did this change: https://codereview.chromium.org/2748023002 > > It touches too many unrelated things to do it in this CL, and also it broke my > Android implementation CL in a possibly-critical way ("class file for > org.chromium.url.mojom.Url not found" -- this might indicate a problem with > Mojo). I'd rather save this change for a follow-up and not try to do it now. Please add a TODO Matt Giuca 2017/03/14 06:53:57 Done. Show quoted text On 2017/03/14 06:41:53, dcheng wrote: > On 2017/03/14 06:38:11, Matt Giuca wrote: > > On 2017/03/13 06:16:51, dcheng wrote: > > > Nit: please make this a mojo.common.mojom.Url. > > > > OK, you're right, this should be a Url. But this is a pretty big refactor > which > > requires also converting the existing WebRelatedApplication's url field from > > WebString to WebUrl (or else the URL gets re-parsed several times). > > > > I did this change: https://codereview.chromium.org/2748023002 > > > > It touches too many unrelated things to do it in this CL, and also it broke my > > Android implementation CL in a possibly-critical way ("class file for > > org.chromium.url.mojom.Url not found" -- this might indicate a problem with > > Mojo). I'd rather save this change for a follow-up and not try to do it now. > > Please add a TODO Done.
	12 string? id;

	13 };

OLD	NEW