Block media loading when AwSettings.setBlockNetworkLoads is true.

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.android_webview;
 
 import android.content.Context;
 import android.content.pm.ActivityInfo;
 import android.graphics.Bitmap;
 import android.graphics.Picture;
@@ skipping 37 matching lines @@
     private AwWebContentsObserver mWebContentsObserver;
 
     private AwContentViewClient mContentViewClient = new AwContentViewClient();
 
     // Last background color reported from the renderer. Holds the sentinal value INVALID_COLOR
     // if not valid.
     private int mCachedRendererBackgroundColor = INVALID_COLOR;
 
     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());
     }
 
     // Alllow injection of the callback thread, for testing.
     public AwContentsClient(Looper looper) {
         mCallbackHelper = new AwContentsClientCallbackHelper(looper, this);
     }
 
     class AwWebContentsObserver extends WebContentsObserverAndroid {
@@ skipping 60 matching lines @@
 
         @Override
         public boolean shouldOverrideKeyEvent(KeyEvent event) {
             return AwContentsClient.this.shouldOverrideKeyEvent(event);
         }
 
         @Override
         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 {
         @Override
         public void onShowCustomView(View view) {
             WebChromeClient.CustomViewCallback cb = new WebChromeClient.CustomViewCallback() {
                 @Override
                 public void onCustomViewHidden() {
                     ContentVideoView contentVideoView = ContentVideoView.getContentVideoView();
                     if (contentVideoView != null)
@@ skipping 142 matching lines @@
     public abstract void onFindResultReceived(int activeMatchOrdinal, int numberOfMatches,
             boolean isDoneCounting);
 
     /**
      * Called whenever there is a new content picture available.
      * @param picture New picture.
      */
     public abstract void onNewPicture(Picture picture);
 
 }