Bug 131921 – [iOS] runtime application checks don't work in WebKit2

• Component: Media
• Status: RESOLVED WONTFIX

[Eric Carlson]

The various application checks in RuntimeApplicationChecksIOS.cpp don't work as expected when running in WebKit2 because they assume [[NSBundle mainBundle] bundleIdentifier]  returns the bundle ID of the host application.

[Eric Carlson]

<rdar://16565372>

[Radar WebKit Bug Importer]

<rdar://problem/16671610>

[Eric Carlson]

Created attachment 229777 [details]
Proposed patch.

[WebKit Commit Bot]

Attachment 229777 [details] did not pass style-queue:


ERROR: Source/WebKit2/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
ERROR: Source/WebKit/mac/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sam Weinig]

Comment on attachment 229777 [details]
Proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=229777&action=review

> Source/WebCore/page/Settings.h:275
> +    static void setEmbeddingApplicationBundleID(const String&);
> +    static const String& embeddingApplicationBundleID();

These don't seem like Settings things.  Why don't we just put all this in RuntimeApplicationChecks.h/mm.

[Eric Carlson]

Created attachment 229778 [details]
Updated patch

[Darin Adler]

Comment on attachment 229778 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229778&action=review

> Source/WebCore/page/Settings.h:275
> +    static void setEmbeddingApplicationBundleID(const String&);
> +    static const String& embeddingApplicationBundleID();

Never sure if such things belong in Settings, but I guess it’s OK.

> Source/WebKit/mac/WebView/WebView.mm:2458
> +    settings.setEmbeddingApplicationBundleID(String([[NSBundle mainBundle] bundleIdentifier]));

I don’t think the explicit String() is needed here. Is it?

[Eric Carlson]

Created attachment 229783 [details]
Updated patch

[Eric Carlson]

(In reply to comment #7)
> (From update of attachment 229778 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229778&action=review
> 
> > Source/WebCore/page/Settings.h:275
> > +    static void setEmbeddingApplicationBundleID(const String&);
> > +    static const String& embeddingApplicationBundleID();
> 
> Never sure if such things belong in Settings, but I guess it’s OK.
> 
> > Source/WebKit/mac/WebView/WebView.mm:2458
> > +    settings.setEmbeddingApplicationBundleID(String([[NSBundle mainBundle] bundleIdentifier]));
> 
> I don’t think the explicit String() is needed here. Is it?

Oops you are right. I copied that from WebKit2 without thinking about whether or not it was necessary.

[Eric Carlson]

Created attachment 229784 [details]
Updated patch

I don’t understand why this is a bug, and I think the proposed fix (whatever it’s fixing) is conceptually wrong.

Of all the apps being tested for, none currently use the WebKit2 API. On what basis are we customizing behavior for apps that aren’t even clients of the framework? Future adopters of the API should get the API’s standard behavior. If that blocks adoption, they can request whatever additional API they need. If we anticipate such needs, we should provide such API.