Block media loading when AwSettings.setBlockNetworkLoads is true.

android_webview/java/src/org/chromium/android_webview/AwContentsClient.java

Index: android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
diff --git a/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java b/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
index 41989dd3434ba116d183e4b38d7c57f656fc5252..173b63ad79ba5dd439cf5752667a41edd859b21c 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
@@ -55,6 +55,8 @@ public abstract class AwContentsClient {
 
     private static final int INVALID_COLOR = 0;
 
+    private AwSettings mSettings;

[joth, 2013/11/01 19:37:33]

probably cleaner to push this in as a member of the inner AwContentViewClient
class

+
     public AwContentsClient() {
         this(Looper.myLooper());
     }
@@ -135,13 +137,22 @@ public abstract class AwContentsClient {
         final public ContentVideoViewClient getContentVideoViewClient() {
             return new AwContentVideoViewClient();
         }
+
+        @Override
+        public boolean shouldBlockMediaRequest(String url) {
+            if (mSettings != null)

[qinmin, 2013/10/31 16:41:26]

nit: I thought java coding style needs {} for if statement

[Ben Murdoch, 2013/10/31 17:26:56]

nit: could make this one line

return mSettings == null ? true : mSettings.getBlockNetworkLoads();

+                return mSettings.getBlockNetworkLoads();
+            else
+                return true;

[mkosiba (inactive), 2013/10/31 17:05:14]

why is blocking the default? Did WebViewClassic behave this way?

[joth, 2013/11/01 19:37:33]

I think the choice is arbitrary - it's not meaningful to have a
AwContentViewClient without the mSettings so we shouldn't even get this call.
(could assert / NPE in this case I guess?)

We could make this more explicit by only constructing AwContentViewClient when
the AwSettings is passed in (and, make AwSettings a member of
AwContentViewClient as mentioned above)
-or we pull AwContentViewClient right out to owned by the AwContents like you
propose below. AwWebContentsDelegateAdapter already went through that latter
process. Maybe that suggests the pattern.

Separate CL?

+        }
     }
 
-    final void installWebContentsObserver(ContentViewCore contentViewCore) {
+    final void installWebContentsObserver(ContentViewCore contentViewCore, AwSettings settings) {

[mkosiba (inactive), 2013/10/31 17:05:14]

method doesn't do what it says anymore. Maybe add a
AwContentsClient.setAwSettings method?

My gut feeling is that this routing is a bit dirty and that we should move
AwContentViewClient to live in AwContents. I don't have any strong feelings
about this though.
+joth@ - WDYT?

         if (mWebContentsObserver != null) {
             mWebContentsObserver.detachFromWebContents();
         }
         mWebContentsObserver = new AwWebContentsObserver(contentViewCore);
+        mSettings = settings;
     }
 
     private class AwContentVideoViewClient implements ContentVideoViewClient {