Restrict getInstalledRelatedApps() to non-incognito contexts.

content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderFactory.java

Index: content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderFactory.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderFactory.java b/content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderFactory.java
index 82bf8be2a8d6c3bd4dea4761cf4a2d89649379d3..6eda1a32f02735d6e74e5891582a5320ecf9b422 100644
--- a/content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderFactory.java
+++ b/content/public/android/java/src/org/chromium/content/browser/installedapp/InstalledAppProviderFactory.java
@@ -5,6 +5,7 @@
 package org.chromium.content.browser.installedapp;
 
 import org.chromium.base.ContextUtils;
+import org.chromium.content.browser.framehost.RenderFrameHostImpl;
 import org.chromium.content_public.browser.RenderFrameHost;
 import org.chromium.installedapp.mojom.InstalledAppProvider;
 import org.chromium.services.service_manager.InterfaceFactory;
@@ -18,10 +19,10 @@ public class InstalledAppProviderFactory implements InterfaceFactory<InstalledAp
 
     private static final class FrameUrlDelegateImpl
             implements InstalledAppProviderImpl.FrameUrlDelegate {
-        private final RenderFrameHost mRenderFrameHost;
+        private final RenderFrameHostImpl mRenderFrameHost;
 
         public FrameUrlDelegateImpl(RenderFrameHost renderFrameHost) {
-            mRenderFrameHost = renderFrameHost;
+            mRenderFrameHost = (RenderFrameHostImpl) renderFrameHost;

[Matt Giuca, 2017/03/24 05:06:55]

Ew, please don't do this. As I said above, add the method to the interface then
you don't need to know anything about Impl.

[Matt Giuca, 2017/03/24 05:31:44]

Discussed offline. Apparently this ... is a thing?

OK go ahead and land this. I think moving this file out of content makes sense
actually, and then we should not be accessing the RFHI, but for now this is OK.

[boliu, 2017/03/24 16:34:46]

Yeah this is one of the things called out in content API:
https://www.chromium.org/developers/content-module/content-api
"""
content implementation code should use other implementations directly and not go
through the interface (i.e. code in content/renderer should use RenderViewImpl
instead of content::RenderView)
"""

[Matt Giuca, 2017/03/27 00:16:29]

Right, but that advice is kind of telling you "if you have a RFHI, don't upcast
it to a RFH and use the interface; keep it as a RFHI". Which is fine.

The gross part is that here we have a RFH abstract reference (going way up the
chain to InterfaceRegistrar.addRenderFrameHostRegistrar) that isn't guaranteed
to be a RFHI, and now we're assuming it's an RFHI and downcasting. Which
violates basically the main law of object oriented programming (if you have a
base class, don't assume it's a subclass).

Apparently I haven't been programming enough in content/ to realise this is a
thing that happens a lot, so I guess we just keep doing it here. But as an
outside observer, this seems like a disaster.

[boliu, 2017/03/27 16:12:27]

The indirection is to hide implementation details and avoid exposing methods to
content embedder. There are is only single implementations the interface
(ignoring tests), so casting should always be safe.

I mean if you want to avoid casting, then can change
ContentRenderFrameHostInterfaceRegistrar to work on RFHI rather than RFH.

         }
 
         @Override
@@ -35,6 +36,11 @@ public class InstalledAppProviderFactory implements InterfaceFactory<InstalledAp
                 throw new AssertionError(e);
             }
         }
+
+        @Override
+        public boolean isIncognito() {
+            return mRenderFrameHost.isIncognito();
+        }
     }
 
     public InstalledAppProviderFactory(RenderFrameHost renderFrameHost) {