Report quality enforcement violation via console message

This CL adds a jni method to let QualityEnforcer notify native and send
a console message for violations.
We will move to use devtools issue tab instead in the near future.
(TODO in quality_enforcer.cc)

The warning together with the toast warning are behind a flag
TRUSTED_WEB_ACTIVITY_QUALITY_ENFORCEMENT_WARNING, just in case we need
to disabled them for any reason.
The flag is *Enabled* for now since we have all approval on launch bug:
crbug/1127892

To have a valid tab and WebContent to send the message to, moves the
check of digital asset link violation to onDidFinishNavigation from
onFinishNativeInitialization.

This also moves the ViolationType enum to c++ part to be able to share
between c++ and java.

Bug: 1147479
Change-Id: If0f80fee412ea522a7202226a72b169092525cef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2530800
Commit-Queue: Ella Ge <eirage@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826701}

chrome/android/BUILD.gn

@@ -709,6 +709,7 @@ java_cpp_enum("chrome_android_java_enums_srcjar") {
     "//chrome/browser/download/android/download_open_source.h",
     "//chrome/browser/installable/digital_asset_links/digital_asset_links_handler.h",
     "//chrome/browser/installable/installed_webapp_geolocation_bridge.cc",
+    "//chrome/browser/installable/quality_enforcer.cc",
     "//chrome/browser/notifications/notification_channels_provider_android.h",
     "//chrome/browser/notifications/notification_handler.h",
     "//chrome/browser/notifications/notification_platform_bridge_android.cc",
@@ -3179,6 +3180,7 @@ generate_jni("chrome_jni_headers") {
     "java/src/org/chromium/chrome/browser/banners/AppBannerManagerHelper.java",
     "java/src/org/chromium/chrome/browser/bookmarks/BookmarkBridge.java",
     "java/src/org/chromium/chrome/browser/browserservices/OriginVerifier.java",
+    "java/src/org/chromium/chrome/browser/browserservices/QualityEnforcer.java",
     "java/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappBridge.java",
     "java/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappGeolocationBridge.java",
     "java/src/org/chromium/chrome/browser/browsing_data/BrowsingDataBridge.java",

chrome/android/java/src/org/chromium/chrome/browser/browserservices/QualityEnforcer.java

@@ -8,13 +8,13 @@ import android.content.Context;
 import android.content.pm.PackageManager;
 import android.os.Bundle;
 
-import androidx.annotation.IntDef;
 import androidx.annotation.NonNull;
 import androidx.annotation.VisibleForTesting;
 import androidx.browser.customtabs.CustomTabsSessionToken;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Promise;
+import org.chromium.base.annotations.NativeMethods;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.app.ChromeActivity;
 import org.chromium.chrome.browser.browserservices.ui.controller.Verifier;
@@ -25,15 +25,12 @@ import org.chromium.chrome.browser.customtabs.content.TabObserverRegistrar.Custo
 import org.chromium.chrome.browser.dependency_injection.ActivityScope;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
-import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.content_public.browser.NavigationHandle;
+import org.chromium.content_public.browser.RenderFrameHost;
 import org.chromium.net.NetError;
 import org.chromium.ui.widget.Toast;
 
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-
 import javax.inject.Inject;
 
 /**
@@ -46,7 +43,7 @@ import javax.inject.Inject;
  * will crash. We should hold web apps to the same standard.
  */
 @ActivityScope
-public class QualityEnforcer implements NativeInitObserver {
+public class QualityEnforcer {
     @VisibleForTesting
     static final String CRASH = "quality_enforcement.crash";
     @VisibleForTesting
@@ -62,21 +59,9 @@ public class QualityEnforcer implements NativeInitObserver {
     private final BrowserServicesIntentDataProvider mIntentDataProvider;
     private final TrustedWebActivityUmaRecorder mUmaRecorder;
 
+    private boolean mFirstNavigationFinished;
     private boolean mOriginVerified;
 
-    // Do not modify or reuse existing entries, they are used in a UMA histogram. Please also edit
-    // TrustedWebActivityQualityEnforcementViolationType in enums.xml if new value added.
-    @IntDef({ViolationType.ERROR_404, ViolationType.ERROR_5XX, ViolationType.UNAVAILABLE_OFFLINE,
-            ViolationType.DIGITAL_ASSETLINKS})
-    @Retention(RetentionPolicy.SOURCE)
-    public @interface ViolationType {
-        int ERROR_404 = 0;
-        int ERROR_5XX = 1;
-        int UNAVAILABLE_OFFLINE = 2;
-        int DIGITAL_ASSETLINKS = 3;
-        int NUM_ENTRIES = 4;
-    }
-
     private final CustomTabTabObserver mTabObserver = new CustomTabTabObserver() {
         @Override
         public void onDidFinishNavigation(Tab tab, NavigationHandle navigation) {
@@ -85,15 +70,29 @@ public class QualityEnforcer implements NativeInitObserver {
                 return;
             }
 
+            if (!mFirstNavigationFinished) {
+                String loadUrl = mIntentDataProvider.getUrlToLoad();
+                mFirstNavigationFinished = true;
+                mVerifier.verify(loadUrl).then((verified) -> {
+                    if (!verified) {
+                        trigger(tab, QualityEnforcementViolationType.DIGITAL_ASSET_LINK,
+                                mIntentDataProvider.getUrlToLoad(), 0);
+                    }
+                });
+            }
+
             String newUrl = tab.getOriginalUrl();
             if (isNavigationInScope(newUrl)) {
                 if (navigation.httpStatusCode() == 404) {
-                    trigger(ViolationType.ERROR_404, newUrl, navigation.httpStatusCode());
+                    trigger(tab, QualityEnforcementViolationType.HTTP_ERROR404, newUrl,
+                            navigation.httpStatusCode());
                 } else if (navigation.httpStatusCode() >= 500
                         && navigation.httpStatusCode() <= 599) {
-                    trigger(ViolationType.ERROR_5XX, newUrl, navigation.httpStatusCode());
+                    trigger(tab, QualityEnforcementViolationType.HTTP_ERROR5XX, newUrl,
+                            navigation.httpStatusCode());
                 } else if (navigation.errorCode() == NetError.ERR_INTERNET_DISCONNECTED) {
-                    trigger(ViolationType.UNAVAILABLE_OFFLINE, newUrl, navigation.httpStatusCode());
+                    trigger(tab, QualityEnforcementViolationType.UNAVAILABLE_OFFLINE, newUrl,
+                            navigation.httpStatusCode());
                 }
             }
         }
@@ -122,25 +121,23 @@ public class QualityEnforcer implements NativeInitObserver {
         // Initialize the value to true before the first navigation.
         mOriginVerified = true;
         tabObserverRegistrar.registerActivityTabObserver(mTabObserver);
-        lifecycleDispatcher.register(this);
     }
 
-    @Override
-    public void onFinishNativeInitialization() {
-        String url = mIntentDataProvider.getUrlToLoad();
-        mVerifier.verify(url).then((verified) -> {
-            if (!verified) {
-                trigger(ViolationType.DIGITAL_ASSETLINKS, mIntentDataProvider.getUrlToLoad(), 0);
-            }
-        });
-    }
-
-    private void trigger(@ViolationType int type, String url, int httpStatusCode) {
+    private void trigger(
+            Tab tab, @QualityEnforcementViolationType int type, String url, int httpStatusCode) {
         mUmaRecorder.recordQualityEnforcementViolation(type, false /* crashed */);
 
+        if (ChromeFeatureList.isEnabled(
+                    ChromeFeatureList.TRUSTED_WEB_ACTIVITY_QUALITY_ENFORCEMENT_WARNING)) {
+            showErrorToast(getToastMessage(type, url, httpStatusCode));
+            if (tab.getWebContents() != null) {
+                QualityEnforcerJni.get().reportDevtoolsIssue(
+                        tab.getWebContents().getMainFrame(), type, url, httpStatusCode);
+            }
+        }
+
         if (!ChromeFeatureList.isEnabled(
                     ChromeFeatureList.TRUSTED_WEB_ACTIVITY_QUALITY_ENFORCEMENT)) {
-            showErrorToast(getToastMessage(type, url, httpStatusCode));
             return;
         }
 
@@ -150,14 +147,9 @@ public class QualityEnforcer implements NativeInitObserver {
         Bundle result = mConnection.sendExtraCallbackWithResult(mSessionToken, CRASH, args);
         boolean success = result != null && result.getBoolean(KEY_SUCCESS);
 
-        // Show the Toast if client app does not enable quality enforcement.
-        if (!success) {
-            showErrorToast(getToastMessage(type, url, httpStatusCode));
-        }
-
         // Do not crash on assetlink failures if the client app does not have installer package
         // name.
-        if (type == ViolationType.DIGITAL_ASSETLINKS && !isDebugInstall()) {
+        if (type == QualityEnforcementViolationType.DIGITAL_ASSET_LINK && !isDebugInstall()) {
             return;
         }
 
@@ -192,16 +184,17 @@ public class QualityEnforcer implements NativeInitObserver {
     }
 
     /* Get the localized string for toast message. */
-    private String getToastMessage(@ViolationType int type, String url, int httpStatusCode) {
+    private String getToastMessage(
+            @QualityEnforcementViolationType int type, String url, int httpStatusCode) {
         switch (type) {
-            case ViolationType.ERROR_404:
-            case ViolationType.ERROR_5XX:
+            case QualityEnforcementViolationType.HTTP_ERROR404:
+            case QualityEnforcementViolationType.HTTP_ERROR5XX:
                 return ContextUtils.getApplicationContext().getString(
                         R.string.twa_quality_enforcement_violation_error, httpStatusCode, url);
-            case ViolationType.UNAVAILABLE_OFFLINE:
+            case QualityEnforcementViolationType.UNAVAILABLE_OFFLINE:
                 return ContextUtils.getApplicationContext().getString(
                         R.string.twa_quality_enforcement_violation_offline, url);
-            case ViolationType.DIGITAL_ASSETLINKS:
+            case QualityEnforcementViolationType.DIGITAL_ASSET_LINK:
                 return ContextUtils.getApplicationContext().getString(
                         R.string.twa_quality_enforcement_violation_asset_link, url);
             default:
@@ -213,14 +206,15 @@ public class QualityEnforcer implements NativeInitObserver {
      * Get the string for sending message to TWA client app. We are not using the localized one as
      * the toast because this is used in TWA's crash message.
      */
-    private String toTwaCrashMessage(@ViolationType int type, String url, int httpStatusCode) {
+    private String toTwaCrashMessage(
+            @QualityEnforcementViolationType int type, String url, int httpStatusCode) {
         switch (type) {
-            case ViolationType.ERROR_404:
-            case ViolationType.ERROR_5XX:
+            case QualityEnforcementViolationType.HTTP_ERROR404:
+            case QualityEnforcementViolationType.HTTP_ERROR5XX:
                 return httpStatusCode + " on " + url;
-            case ViolationType.UNAVAILABLE_OFFLINE:
+            case QualityEnforcementViolationType.UNAVAILABLE_OFFLINE:
                 return "Page unavailable offline: " + url;
-            case ViolationType.DIGITAL_ASSETLINKS:
+            case QualityEnforcementViolationType.DIGITAL_ASSET_LINK:
                 return "Digital asset links verification failed on " + url;
             default:
                 return "";
@@ -235,4 +229,10 @@ public class QualityEnforcer implements NativeInitObserver {
                        mClientPackageNameProvider.get())
                 != null;
     }
+
+    @NativeMethods
+    interface Natives {
+        void reportDevtoolsIssue(
+                RenderFrameHost renderFrameHost, int type, String url, int httpStatusCode);
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/browserservices/TrustedWebActivityUmaRecorder.java

@@ -215,13 +215,13 @@ public class TrustedWebActivityUmaRecorder {
     }
 
     public void recordQualityEnforcementViolation(
-            @QualityEnforcer.ViolationType int type, boolean crashed) {
+            @QualityEnforcementViolationType int type, boolean crashed) {
         RecordHistogram.recordEnumeratedHistogram("TrustedWebActivity.QualityEnforcementViolation",
-                type, QualityEnforcer.ViolationType.NUM_ENTRIES);
+                type, QualityEnforcementViolationType.MAX_VALUE + 1);
         if (crashed) {
             RecordHistogram.recordEnumeratedHistogram(
                     "TrustedWebActivity.QualityEnforcementViolation.Crashed", type,
-                    QualityEnforcer.ViolationType.NUM_ENTRIES);
+                    QualityEnforcementViolationType.MAX_VALUE + 1);
         }
     }
 }

chrome/android/junit/src/org/chromium/chrome/browser/browserservices/QualityEnforcerUnitTest.java

@@ -30,6 +30,7 @@ import org.robolectric.shadows.ShadowToast;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Promise;
 import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.base.test.util.JniMocker;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.app.ChromeActivity;
 import org.chromium.chrome.browser.browserservices.ui.controller.Verifier;
@@ -54,7 +55,8 @@ import org.chromium.url.JUnitTestGURLs;
  */
 @RunWith(BaseRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
-@EnableFeatures(ChromeFeatureList.TRUSTED_WEB_ACTIVITY_QUALITY_ENFORCEMENT)
+@EnableFeatures({ChromeFeatureList.TRUSTED_WEB_ACTIVITY_QUALITY_ENFORCEMENT,
+        ChromeFeatureList.TRUSTED_WEB_ACTIVITY_QUALITY_ENFORCEMENT_WARNING})
 @DisableFeatures(ChromeFeatureList.TRUSTED_WEB_ACTIVITY_QUALITY_ENFORCEMENT_FORCED)
 public class QualityEnforcerUnitTest {
     private static final GURL TRUSTED_ORIGIN_PAGE = JUnitTestGURLs.getGURL(JUnitTestGURLs.URL_1);
@@ -64,6 +66,10 @@ public class QualityEnforcerUnitTest {
 
     @Rule
     public TestRule mFeaturesProcessor = new Features.JUnitProcessor();
+
+    @Rule
+    public JniMocker mocker = new JniMocker();
+
     @Mock
     private ChromeActivity mActivity;
     @Mock
@@ -84,6 +90,8 @@ public class QualityEnforcerUnitTest {
     private Tab mTab;
     @Mock
     public TrustedWebActivityUmaRecorder mUmaRecorder;
+    @Mock
+    private QualityEnforcer.Natives mNativeMock;
 
     private ShadowPackageManager mShadowPackageManager;
 
@@ -92,6 +100,8 @@ public class QualityEnforcerUnitTest {
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
+        mocker.mock(QualityEnforcerJni.TEST_HOOKS, mNativeMock);
+
         doNothing()
                 .when(mTabObserverRegistrar)
                 .registerActivityTabObserver(mTabObserverCaptor.capture());
@@ -102,6 +112,7 @@ public class QualityEnforcerUnitTest {
         mQualityEnforcer = new QualityEnforcer(mActivity, mLifecycleDispatcher,
                 mTabObserverRegistrar, mIntentDataProvider, mCustomTabsConnection, mVerifier,
                 mClientPackageNameProvider, mUmaRecorder);
+        when(mIntentDataProvider.getUrlToLoad()).thenReturn(TRUSTED_ORIGIN_PAGE.getSpec());
     }
 
     @Test
@@ -163,14 +174,14 @@ public class QualityEnforcerUnitTest {
     @Test
     public void notTrigger_digitalAssetLinkPass() {
         when(mIntentDataProvider.getUrlToLoad()).thenReturn(TRUSTED_ORIGIN_PAGE.getSpec());
-        mQualityEnforcer.onFinishNativeInitialization();
+        navigateToUrlNoError(TRUSTED_ORIGIN_PAGE);
         verifyNotTriggered();
     }
 
     @Test
     public void trigger_digitalAssetLinkFailed() {
         when(mIntentDataProvider.getUrlToLoad()).thenReturn(UNTRUSTED_PAGE.getSpec());
-        mQualityEnforcer.onFinishNativeInitialization();
+        navigateToUrlNoError(UNTRUSTED_PAGE);
         verifyToastShown(ContextUtils.getApplicationContext().getString(
                 R.string.twa_quality_enforcement_violation_asset_link, UNTRUSTED_PAGE.getSpec()));
         verifyNotifyClientApp();
@@ -197,7 +208,7 @@ public class QualityEnforcerUnitTest {
     public void triggerNotCrash_whenDigitalAssetLinkFailed() {
         setClientEnable(true);
         when(mIntentDataProvider.getUrlToLoad()).thenReturn(UNTRUSTED_PAGE.getSpec());
-        mQualityEnforcer.onFinishNativeInitialization();
+        navigateToUrlNoError(UNTRUSTED_PAGE);
         verifyNotifyClientApp();
         verify(mActivity, never()).finish();
     }

chrome/browser/BUILD.gn

@@ -2978,6 +2978,7 @@ static_library("browser") {
       "installable/installed_webapp_geolocation_context.h",
       "installable/installed_webapp_provider.cc",
       "installable/installed_webapp_provider.h",
+      "installable/quality_enforcer.cc",
       "lifetime/application_lifetime_android.cc",
       "lifetime/application_lifetime_android.h",
       "media/android/cdm/media_drm_origin_id_manager.cc",

chrome/browser/flags/android/chrome_feature_list.cc

@@ -221,6 +221,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
     &kTrustedWebActivityPostMessage,
     &kTrustedWebActivityQualityEnforcement,
     &kTrustedWebActivityQualityEnforcementForced,
+    &kTrustedWebActivityQualityEnforcementWarning,
     &kStartSurfaceAndroid,
     &kUmaBackgroundSessions,
     &kUpdateNotificationSchedulingIntegration,
@@ -641,6 +642,10 @@ const base::Feature kTrustedWebActivityQualityEnforcementForced{
     "TrustedWebActivityQualityEnforcementForced",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kTrustedWebActivityQualityEnforcementWarning{
+    "TrustedWebActivityQualityEnforcementWarning",
+    base::FEATURE_ENABLED_BY_DEFAULT};
+
 const base::Feature kStartSurfaceAndroid{"StartSurfaceAndroid",
                                          base::FEATURE_DISABLED_BY_DEFAULT};
 

chrome/browser/flags/android/chrome_feature_list.h

@@ -129,6 +129,7 @@ extern const base::Feature kTrustedWebActivityNewDisclosure;
 extern const base::Feature kTrustedWebActivityPostMessage;
 extern const base::Feature kTrustedWebActivityQualityEnforcement;
 extern const base::Feature kTrustedWebActivityQualityEnforcementForced;
+extern const base::Feature kTrustedWebActivityQualityEnforcementWarning;
 extern const base::Feature kStartSurfaceAndroid;
 extern const base::Feature kUmaBackgroundSessions;
 extern const base::Feature kUpdateNotificationSchedulingIntegration;

chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java

@@ -443,6 +443,8 @@ public abstract class ChromeFeatureList {
             "TrustedWebActivityQualityEnforcement";
     public static final String TRUSTED_WEB_ACTIVITY_QUALITY_ENFORCEMENT_FORCED =
             "TrustedWebActivityQualityEnforcementForced";
+    public static final String TRUSTED_WEB_ACTIVITY_QUALITY_ENFORCEMENT_WARNING =
+            "TrustedWebActivityQualityEnforcementWarning";
     public static final String VIDEO_TUTORIALS = "VideoTutorials";
     public static final String UPDATE_NOTIFICATION_SCHEDULING_INTEGRATION =
             "UpdateNotificationSchedulingIntegration";

chrome/browser/installable/quality_enforcer.cc

+// Copyright 2020 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.
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/android/jni_utils.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/android/chrome_jni_headers/QualityEnforcer_jni.h"
+#include "content/public/browser/render_frame_host.h"
+#include "third_party/blink/public/mojom/devtools/console_message.mojom.h"
+#include "url/gurl.h"
+
+using base::android::JavaParamRef;
+
+namespace {
+
+// Do not modify or reuse existing entries; they are used in a UMA histogram.
+// Please also edit TrustedWebActivityQualityEnforcementViolationType in the
+// enums.xml if adding new value.
+// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.browserservices
+enum class QualityEnforcementViolationType {
+  kHttpError404 = 0,
+  kHttpError5xx = 1,
+  kUnavailableOffline = 2,
+  kDigitalAssetLink = 3,
+  kMaxValue = kDigitalAssetLink
+};
+
+constexpr char kHttpErrorConsoleMessageFormat[] =
+    "HTTP error %d when navigating to %s. Please make sure your "
+    "app doesn’t have 404 or 5xx errors.";
+
+constexpr char kUnavailableOfflineConsoleMessageFormat[] =
+    "Page %s is not available offline. Please handle offline resource requests "
+    "using a ServiceWorker. See: "
+    "https://developers.google.com/web/fundamentals/codelabs/offline";
+
+constexpr char kDigitalAssetLinkConsoleMessageFormat[] =
+    "Launch URL %s failed digital asset link verification. Please review the "
+    "Trusted Web Activity quick start guide for how to correctly implement "
+    "Digital Assetlinks: "
+    "https://developers.google.com/web/android/trusted-web-activity/"
+    "quick-start";
+
+}  // namespace
+
+static void JNI_QualityEnforcer_ReportDevtoolsIssue(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& jrender_frame_host,
+    int type,
+    const JavaParamRef<jstring>& jurl,
+    int http_error_code) {
+  auto* render_frame_host =
+      content::RenderFrameHost::FromJavaRenderFrameHost(jrender_frame_host);
+  if (!render_frame_host)  // The frame is being unloaded.
+    return;
+
+  std::string url = base::android::ConvertJavaStringToUTF8(env, jurl);
+
+  // TODO(crbug.com/1147479): Report the message in devtools issue tab instead.
+  switch (static_cast<QualityEnforcementViolationType>(type)) {
+    case QualityEnforcementViolationType::kHttpError404:
+    case QualityEnforcementViolationType::kHttpError5xx:
+      render_frame_host->AddMessageToConsole(
+          blink::mojom::ConsoleMessageLevel::kWarning,
+          base::StringPrintf(kHttpErrorConsoleMessageFormat, http_error_code,
+                             url.c_str()));
+      return;
+    case QualityEnforcementViolationType::kUnavailableOffline:
+      render_frame_host->AddMessageToConsole(
+          blink::mojom::ConsoleMessageLevel::kWarning,
+          base::StringPrintf(kUnavailableOfflineConsoleMessageFormat,
+                             url.c_str()));
+      return;
+    case QualityEnforcementViolationType::kDigitalAssetLink:
+      render_frame_host->AddMessageToConsole(
+          blink::mojom::ConsoleMessageLevel::kWarning,
+          base::StringPrintf(kDigitalAssetLinkConsoleMessageFormat,
+                             url.c_str()));
+      return;
+  }
+}