New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 687444 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Long OOO (go/where-is-mgiuca)
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

IDL: RelatedApplication should be a dictionary, not an interface

Project Member Reported by mgiuca@chromium.org, Feb 1 2017

Issue description

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/installedapp/RelatedApplication.idl

RelatedApplication is defined as:
interface RelatedApplication {
    readonly attribute DOMString platform;
    readonly attribute DOMString url;
    readonly attribute DOMString id;
};

There is really no reason for this to be an interface; it can just be a simple dictionary (like ShareData).

I tried converting it and it was a big deal (the code that binds to it needs to completely change and there are garbage collector ramifications). Still, this should be changed before it is spec'd.

In the mean time, I think we can add [NoInterfaceObject] to remove the presence of the top-level interface object RelatedApplication.

Note: The current implementation is impossible to idlharness-test due to Issue 643712. That isn't a good reason to change it, but I think changing it (for other reasons) will work around that.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/952ec0e36758c881a1e7a388895eb61d0d840817

commit 952ec0e36758c881a1e7a388895eb61d0d840817
Author: mgiuca <mgiuca@chromium.org>
Date: Thu Feb 02 07:40:12 2017

Some small tweaks to getInstalledRelatedApps IDL files.

User-visible change: RelatedApplication is no longer a visible
interface. It doesn't need to be (and in fact should probably just be a
dictionary). Only affects an unreleased API behind a flag.

Also adds an idlharness test that runs basic tests over the interface.
Also updated comments and added TODOs.

BUG=687444

Review-Url: https://codereview.chromium.org/2662303003
Cr-Commit-Position: refs/heads/master@{#447720}

[add] https://crrev.com/952ec0e36758c881a1e7a388895eb61d0d840817/third_party/WebKit/LayoutTests/installedapp/idl.html
[modify] https://crrev.com/952ec0e36758c881a1e7a388895eb61d0d840817/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/952ec0e36758c881a1e7a388895eb61d0d840817/third_party/WebKit/Source/modules/installedapp/NavigatorInstalledApp.idl
[modify] https://crrev.com/952ec0e36758c881a1e7a388895eb61d0d840817/third_party/WebKit/Source/modules/installedapp/RelatedApplication.idl

Components: UI>Browser>WebAppInstalls

Sign in to add a comment