Revert "Reland "Injectable logging""

This reverts commit 21219a0e43.

Reason for revert: No tags are written when routing logs to a file - b/86953692

Original change's description:
> Reland "Injectable logging"
> 
> Any injected loggable or NativeLogger would be deleted if PCFactory
> was reinitialized without calling setInjectableLogger. Now native
> logging is not implemented as a Loggable, so it will remain active
> unless a Loggable is injected.
> 
> This is a reland of 59216ec4a4
> 
> Original change's description:
> > Injectable logging
> >
> > Allows passing a Loggable to PCFactory.initializationOptions, which
> > is then injected to Logging.java and logging.h. Future log messages
> > in both Java and native will then be passed to this Loggable.
> >
> > Bug: webrtc:9225
> > Change-Id: I2ff693380639448301a78a93dc11d3a0106f0967
> > Reviewed-on: https://webrtc-review.googlesource.com/73243
> > Commit-Queue: Paulina Hensman <phensman@webrtc.org>
> > Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
> > Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#23241}
> 
> Bug: webrtc:9225
> Change-Id: I2fe3fbc8c323814284bb62e43fe1870bdab581ee
> TBR: kwiberg
> Reviewed-on: https://webrtc-review.googlesource.com/77140
> Commit-Queue: Paulina Hensman <phensman@webrtc.org>
> Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#23310}

TBR=magjed@webrtc.org,sakal@webrtc.org,kwiberg@webrtc.org,phensman@webrtc.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:9225
Change-Id: I4d0a5990b5f742cc1a96afde3ca97fad9143d2d4
Reviewed-on: https://webrtc-review.googlesource.com/80641
Reviewed-by: Alex Glaznev <glaznev@webrtc.org>
Commit-Queue: Alex Glaznev <glaznev@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23498}

rtc_base/BUILD.gn

@@ -1315,7 +1315,6 @@ if (is_android) {
   rtc_android_library("base_java") {
     java_files = [
       "java/src/org/webrtc/ContextUtils.java",
-      "java/src/org/webrtc/Loggable.java",
       "java/src/org/webrtc/Logging.java",
       "java/src/org/webrtc/Size.java",
       "java/src/org/webrtc/ThreadUtils.java",

rtc_base/java/src/org/webrtc/Loggable.java

@@ -1,22 +0,0 @@
-/*
- *  Copyright (c) 2018 The WebRTC project authors. All Rights Reserved.
- *
- *  Use of this source code is governed by a BSD-style license
- *  that can be found in the LICENSE file in the root of the source
- *  tree. An additional intellectual property rights grant can be found
- *  in the file PATENTS.  All contributing project authors may
- *  be found in the AUTHORS file in the root of the source tree.
- */
-
-package org.webrtc;
-
-import org.webrtc.Logging.Severity;
-
-/**
- * Java interface for WebRTC logging. The default implementation uses webrtc.Logging.
- *
- * When injected, the Loggable will receive logging from both Java and native.
- */
-public interface Loggable {
-  public void onLogMessage(String message, Severity severity, String tag);
-}

rtc_base/java/src/org/webrtc/Logging.java

@@ -15,35 +15,21 @@ import java.io.StringWriter;
 import java.util.EnumSet;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import javax.annotation.Nullable;
-import org.webrtc.Loggable;
 
 /**
- * Java wrapper for WebRTC logging. Logging defaults to java.util.logging.Logger, but a custom
+ * Java wrapper for WebRTC logging. Logging defaults to java.util.logging.Logger, but will switch to
- * logger implementing the Loggable interface can be injected along with a Severity. All subsequent
+ * native logging (rtc::LogMessage) if one of the following static functions are called from the
- * log messages will then be redirected to the injected Loggable, except those with a severity lower
+ * app:
- * than the specified severity, which will be discarded.
- *
- * It is also possible to switch to native logging (rtc::LogMessage) if one of the following static
- * functions are called from the app:
  * - Logging.enableLogThreads
  * - Logging.enableLogTimeStamps
  * - Logging.enableLogToDebugOutput
  *
- * The priority goes:
+ * Using these APIs requires that the native library is loaded, using
- * 1. Injected loggable
+ * PeerConnectionFactory.initialize.
- * 2. Native logging
- * 3. Fallback logging.
- * Only one method will be used at a time.
- *
- * Injecting a Loggable or using any of the enable... methods requires that the native library is
- * loaded, using PeerConnectionFactory.initialize.
  */
 public class Logging {
   private static final Logger fallbackLogger = createFallbackLogger();
   private static volatile boolean loggingEnabled;
-  @Nullable private static Loggable loggable;
-  private static Severity loggableSeverity;
 
   private static Logger createFallbackLogger() {
     final Logger fallbackLogger = Logger.getLogger("org.webrtc.Logging");
@@ -51,17 +37,6 @@ public class Logging {
     return fallbackLogger;
   }
 
-  static void injectLoggable(Loggable injectedLoggable, Severity severity) {
-    if (injectedLoggable != null) {
-      loggable = injectedLoggable;
-      loggableSeverity = severity;
-    }
-  }
-
-  static void deleteInjectedLoggable() {
-    loggable = null;
-  }
-
   // TODO(solenberg): Remove once dependent projects updated.
   @Deprecated
   public enum TraceLevel {
@@ -108,26 +83,11 @@ public class Logging {
   // TODO(bugs.webrtc.org/8491): Remove NoSynchronizedMethodCheck suppression.
   @SuppressWarnings("NoSynchronizedMethodCheck")
   public static synchronized void enableLogToDebugOutput(Severity severity) {
-    if (loggable != null) {
-      throw new IllegalStateException(
-          "Logging to native debug output not supported while Loggable is injected. "
-          + "Delete the Loggable before calling this method.");
-    }
     nativeEnableLogToDebugOutput(severity.ordinal());
     loggingEnabled = true;
   }
 
   public static void log(Severity severity, String tag, String message) {
-    if (loggable != null) {
-      // Filter log messages below loggableSeverity.
-      if (severity.ordinal() < loggableSeverity.ordinal()) {
-        return;
-      }
-      loggable.onLogMessage(message, severity, tag);
-      return;
-    }
-
-    // Try native logging if no loggable is injected.
     if (loggingEnabled) {
       nativeLog(severity.ordinal(), tag, message);
       return;

rtc_base/logging.cc

@@ -81,11 +81,6 @@ std::ostream& GetNoopStream() {
 CriticalSection g_log_crit;
 }  // namespace
 
-void LogSink::OnLogMessage(const std::string& msg,
-                           LoggingSeverity severity,
-                           const char* tag) {
-  OnLogMessage(msg);
-}
 
 /////////////////////////////////////////////////////////////////////////////
 // LogMessage
@@ -132,14 +127,8 @@ LogMessage::LogMessage(const char* file,
     print_stream_ << "[" << std::dec << id << "] ";
   }
 
-  if (file != nullptr) {
+  if (file != nullptr)
-#if defined(WEBRTC_ANDROID)
-    tag_ = FilenameFromPath(file);
-    print_stream_ << "(line " << line << "): ";
-#else
     print_stream_ << "(" << FilenameFromPath(file)  << ":" << line << "): ";
-#endif
-  }
 
   if (err_ctx != ERRCTX_NONE) {
     char tmp_buf[1024];
@@ -192,6 +181,7 @@ LogMessage::LogMessage(const char* file,
                  0 /* err */) {
   if (!is_noop_) {
     tag_ = tag;
+    print_stream_ << tag << ": ";
   }
 }
 #endif
@@ -229,11 +219,7 @@ LogMessage::~LogMessage() {
   CritScope cs(&g_log_crit);
   for (auto& kv : streams_) {
     if (severity_ >= kv.second) {
-#if defined(WEBRTC_ANDROID)
-      kv.first->OnLogMessage(str, severity_, tag_);
-#else
       kv.first->OnLogMessage(str);
-#endif
     }
   }
 }

rtc_base/logging.h

@@ -116,9 +116,6 @@ class LogSink {
  public:
   LogSink() {}
   virtual ~LogSink() {}
-  virtual void OnLogMessage(const std::string& msg,
-                            LoggingSeverity severity,
-                            const char* tag);
   virtual void OnLogMessage(const std::string& message) = 0;
 };
 
@@ -496,7 +493,7 @@ class LogMessage {
   LoggingSeverity severity_;
 
 #if defined(WEBRTC_ANDROID)
-  // The default Android debug output tag.
+  // The Android debug output tag.
   const char* tag_ = "libjingle";
 #endif
 

rtc_base/logging_unittest.cc

@@ -47,9 +47,6 @@ class LogMessageForTesting : public LogMessage {
 
   const std::string& get_extra() const { return extra_; }
   bool is_noop() const { return is_noop_; }
-#if defined(WEBRTC_ANDROID)
-  const char* get_tag() const { return tag_; }
-#endif
 
   // Returns the contents of the internal log stream.
   // Note that parts of the stream won't (as is) be available until *after* the
@@ -189,13 +186,7 @@ TEST(LogTest, CheckFilePathParsed) {
   log_msg.stream() << "<- Does this look right?";
 
   const std::string stream = log_msg.GetPrintStream();
-#if defined(WEBRTC_ANDROID)
-  const char* tag = log_msg.get_tag();
-  EXPECT_NE(nullptr, strstr(tag, "myfile.cc"));
-  EXPECT_NE(std::string::npos, stream.find("100"));
-#else
   EXPECT_NE(std::string::npos, stream.find("(myfile.cc:100)"));
-#endif
 }
 
 TEST(LogTest, CheckNoopLogEntry) {

sdk/android/BUILD.gn

@@ -90,7 +90,6 @@ rtc_source_set("base_jni") {
     "src/jni/jni_helpers.cc",
     "src/jni/jni_helpers.h",
     "src/jni/pc/audio.h",
-    "src/jni/pc/logging.cc",
     "src/jni/pc/media.h",
     "src/jni/pc/video.h",
   ]
@@ -520,38 +519,6 @@ generate_jni("generated_peerconnection_jni") {
   jni_generator_include = "//sdk/android/src/jni/jni_generator_helper.h"
 }
 
-rtc_android_library("logging_java") {
-  java_files = [ "src/java/org/webrtc/JNILogging.java" ]
-
-  deps = [
-    ":base_java",
-    "//rtc_base:base_java",
-  ]
-}
-
-generate_jni("generated_logging_jni") {
-  sources = [
-    "src/java/org/webrtc/JNILogging.java",
-  ]
-  jni_package = ""
-  jni_generator_include = "//sdk/android/src/jni/jni_generator_helper.h"
-}
-
-rtc_static_library("logging_jni") {
-  visibility = [ "*" ]
-  sources = [
-    "src/jni/logging/logsink.cc",
-    "src/jni/logging/logsink.h",
-  ]
-
-  deps = [
-    ":base_jni",
-    ":generated_logging_jni",
-    ":native_api_jni",
-    "../../rtc_base:rtc_base",
-  ]
-}
-
 rtc_static_library("peerconnection_jni") {
   # Do not depend on this target externally unless you absolute have to. It is
   # made public because we don't have a proper NDK yet. Header APIs here are not
@@ -568,6 +535,7 @@ rtc_static_library("peerconnection_jni") {
     "src/jni/pc/dtmfsender.cc",
     "src/jni/pc/icecandidate.cc",
     "src/jni/pc/icecandidate.h",
+    "src/jni/pc/logging.cc",
     "src/jni/pc/mediaconstraints.cc",
     "src/jni/pc/mediaconstraints.h",
     "src/jni/pc/mediasource.cc",
@@ -625,7 +593,6 @@ rtc_static_library("peerconnection_jni") {
     ":base_jni",
     ":generated_external_classes_jni",
     ":generated_peerconnection_jni",
-    ":logging_jni",
     ":native_api_jni",
     "../..:webrtc_common",
     "../../api:libjingle_peerconnection_api",
@@ -752,7 +719,6 @@ dist_jar("libwebrtc") {
     ":java_audio_device_module_java",
     ":libjingle_peerconnection_java",
     ":libjingle_peerconnection_metrics_default_java",
-    ":logging_java",
     ":peerconnection_java",
     ":screencapturer_java",
     ":surfaceviewrenderer_java",
@@ -1047,7 +1013,6 @@ rtc_android_library("peerconnection_java") {
   deps = [
     ":audio_api_java",
     ":base_java",
-    ":logging_java",
     ":video_api_java",
     ":video_java",
     "//modules/audio_device:audio_device_java",

sdk/android/api/org/webrtc/PeerConnectionFactory.java

@@ -13,7 +13,6 @@ package org.webrtc;
 import android.content.Context;
 import java.util.List;
 import javax.annotation.Nullable;
-import org.webrtc.Logging.Severity;
 import org.webrtc.audio.AudioDeviceModule;
 import org.webrtc.audio.LegacyAudioDeviceModule;
 
@@ -42,20 +41,15 @@ public class PeerConnectionFactory {
     final boolean enableInternalTracer;
     final boolean enableVideoHwAcceleration;
     final NativeLibraryLoader nativeLibraryLoader;
-    @Nullable Loggable loggable;
-    @Nullable Severity loggableSeverity;
 
     private InitializationOptions(Context applicationContext, String fieldTrials,
         boolean enableInternalTracer, boolean enableVideoHwAcceleration,
-        NativeLibraryLoader nativeLibraryLoader, @Nullable Loggable loggable,
+        NativeLibraryLoader nativeLibraryLoader) {
-        @Nullable Severity loggableSeverity) {
       this.applicationContext = applicationContext;
       this.fieldTrials = fieldTrials;
       this.enableInternalTracer = enableInternalTracer;
       this.enableVideoHwAcceleration = enableVideoHwAcceleration;
       this.nativeLibraryLoader = nativeLibraryLoader;
-      this.loggable = loggable;
-      this.loggableSeverity = loggableSeverity;
     }
 
     public static Builder builder(Context applicationContext) {
@@ -68,8 +62,6 @@ public class PeerConnectionFactory {
       private boolean enableInternalTracer = false;
       private boolean enableVideoHwAcceleration = true;
       private NativeLibraryLoader nativeLibraryLoader = new NativeLibrary.DefaultLoader();
-      @Nullable private Loggable loggable = null;
-      @Nullable private Severity loggableSeverity = null;
 
       Builder(Context applicationContext) {
         this.applicationContext = applicationContext;
@@ -95,16 +87,9 @@ public class PeerConnectionFactory {
         return this;
       }
 
-      public Builder setInjectableLogger(Loggable loggable, Severity severity) {
-        this.loggable = loggable;
-        this.loggableSeverity = severity;
-        return this;
-      }
-
       public PeerConnectionFactory.InitializationOptions createInitializationOptions() {
         return new PeerConnectionFactory.InitializationOptions(applicationContext, fieldTrials,
-            enableInternalTracer, enableVideoHwAcceleration, nativeLibraryLoader, loggable,
+            enableInternalTracer, enableVideoHwAcceleration, nativeLibraryLoader);
-            loggableSeverity);
       }
     }
   }
@@ -214,16 +199,6 @@ public class PeerConnectionFactory {
     if (options.enableInternalTracer && !internalTracerInitialized) {
       initializeInternalTracer();
     }
-    if (options.loggable != null) {
-      Logging.injectLoggable(options.loggable, options.loggableSeverity);
-      nativeInjectLoggable(new JNILogging(options.loggable), options.loggableSeverity.ordinal());
-    } else {
-      Logging.d(TAG,
-          "PeerConnectionFactory was initialized without an injected Loggable. "
-              + "Any existing Loggable will be deleted.");
-      Logging.deleteInjectedLoggable();
-      nativeDeleteLoggable();
-    }
   }
 
   private void checkInitializeHasBeenCalled() {
@@ -499,6 +474,4 @@ public class PeerConnectionFactory {
   private static native void nativeInvokeThreadsCallbacks(long factory);
   private static native void nativeFreeFactory(long factory);
   private static native long nativeGetNativePeerConnectionFactory(long factory);
-  private static native void nativeInjectLoggable(JNILogging jniLogging, int severity);
-  private static native void nativeDeleteLoggable();
 }

sdk/android/src/java/org/webrtc/JNILogging.java

@@ -1,28 +0,0 @@
-/*
- *  Copyright 2018 The WebRTC Project Authors. All rights reserved.
- *
- *  Use of this source code is governed by a BSD-style license
- *  that can be found in the LICENSE file in the root of the source
- *  tree. An additional intellectual property rights grant can be found
- *  in the file PATENTS.  All contributing project authors may
- *  be found in the AUTHORS file in the root of the source tree.
- */
-
-package org.webrtc;
-
-import org.webrtc.CalledByNative;
-import org.webrtc.Loggable;
-import org.webrtc.Logging.Severity;
-
-class JNILogging {
-  private final Loggable loggable;
-
-  public JNILogging(Loggable loggable) {
-    this.loggable = loggable;
-  }
-
-  @CalledByNative
-  public void logToInjectable(String message, Integer severity, String tag) {
-    loggable.onLogMessage(message, Severity.values()[severity], tag);
-  }
-}

sdk/android/src/jni/logging/logsink.cc

@@ -1,35 +0,0 @@
-/*
- *  Copyright 2018 The WebRTC Project Authors. All rights reserved.
- *
- *  Use of this source code is governed by a BSD-style license
- *  that can be found in the LICENSE file in the root of the source
- *  tree. An additional intellectual property rights grant can be found
- *  in the file PATENTS.  All contributing project authors may
- *  be found in the AUTHORS file in the root of the source tree.
- */
-#include "sdk/android/src/jni/logging/logsink.h"
-
-#include "sdk/android/generated_logging_jni/jni/JNILogging_jni.h"
-
-namespace webrtc {
-namespace jni {
-
-JNILogSink::JNILogSink(JNIEnv* env, const JavaRef<jobject>& j_logging)
-    : j_logging_(env, j_logging) {}
-JNILogSink::~JNILogSink() = default;
-
-void JNILogSink::OnLogMessage(const std::string& msg,
-                              rtc::LoggingSeverity severity,
-                              const char* tag) {
-  JNIEnv* env = AttachCurrentThreadIfNeeded();
-  Java_JNILogging_logToInjectable(env, j_logging_, NativeToJavaString(env, msg),
-                                  NativeToJavaInteger(env, severity),
-                                  NativeToJavaString(env, tag));
-}
-
-void JNILogSink::OnLogMessage(const std::string& msg) {
-  RTC_NOTREACHED();
-}
-
-}  // namespace jni
-}  // namespace webrtc

sdk/android/src/jni/logging/logsink.h

@@ -1,39 +0,0 @@
-/*
- *  Copyright 2018 The WebRTC project authors. All Rights Reserved.
- *
- *  Use of this source code is governed by a BSD-style license
- *  that can be found in the LICENSE file in the root of the source
- *  tree. An additional intellectual property rights grant can be found
- *  in the file PATENTS.  All contributing project authors may
- *  be found in the AUTHORS file in the root of the source tree.
- */
-#ifndef SDK_ANDROID_SRC_JNI_LOGGING_LOGSINK_H_
-#define SDK_ANDROID_SRC_JNI_LOGGING_LOGSINK_H_
-
-#include <string>
-
-#include "rtc_base/logging.h"
-#include "sdk/android/native_api/jni/java_types.h"
-#include "sdk/android/src/jni/jni_helpers.h"
-
-namespace webrtc {
-namespace jni {
-
-class JNILogSink : public rtc::LogSink {
- public:
-  JNILogSink(JNIEnv* env, const JavaRef<jobject>& j_logging);
-  ~JNILogSink() override;
-
-  void OnLogMessage(const std::string& msg,
-                    rtc::LoggingSeverity severity,
-                    const char* tag) override;
-  void OnLogMessage(const std::string& msg) override;
-
- private:
-  const ScopedJavaGlobalRef<jobject> j_logging_;
-};
-
-}  // namespace jni
-}  // namespace webrtc
-
-#endif  // SDK_ANDROID_SRC_JNI_LOGGING_LOGSINK_H_

sdk/android/src/jni/pc/peerconnectionfactory.cc

@@ -27,7 +27,6 @@
 #include "sdk/android/generated_peerconnection_jni/jni/PeerConnectionFactory_jni.h"
 #include "sdk/android/native_api/jni/java_types.h"
 #include "sdk/android/src/jni/jni_helpers.h"
-#include "sdk/android/src/jni/logging/logsink.h"
 #include "sdk/android/src/jni/pc/androidnetworkmonitor.h"
 #include "sdk/android/src/jni/pc/audio.h"
 #include "sdk/android/src/jni/pc/icecandidate.h"
@@ -82,9 +81,6 @@ static char* field_trials_init_string = nullptr;
 static bool factory_static_initialized = false;
 static bool video_hw_acceleration_enabled = true;
 
-// Set in PeerConnectionFactory_InjectLoggable().
-static std::unique_ptr<JNILogSink> jni_log_sink;
-
 void PeerConnectionFactoryNetworkThreadReady() {
   RTC_LOG(LS_INFO) << "Network thread JavaCallback";
   JNIEnv* env = AttachCurrentThreadIfNeeded();
@@ -507,29 +503,5 @@ static jlong JNI_PeerConnectionFactory_GetNativePeerConnectionFactory(
   return jlongFromPointer(factoryFromJava(native_factory));
 }
 
-static void JNI_PeerConnectionFactory_InjectLoggable(
-    JNIEnv* jni,
-    const JavaParamRef<jclass>&,
-    const JavaParamRef<jobject>& j_logging,
-    jint nativeSeverity) {
-  // If there is already a LogSink, remove it from LogMessage.
-  if (jni_log_sink) {
-    rtc::LogMessage::RemoveLogToStream(jni_log_sink.get());
-  }
-  jni_log_sink = rtc::MakeUnique<JNILogSink>(jni, j_logging);
-  rtc::LogMessage::AddLogToStream(
-      jni_log_sink.get(), static_cast<rtc::LoggingSeverity>(nativeSeverity));
-  rtc::LogMessage::LogToDebug(rtc::LS_NONE);
-}
-
-static void JNI_PeerConnectionFactory_DeleteLoggable(
-    JNIEnv* jni,
-    const JavaParamRef<jclass>&) {
-  if (jni_log_sink) {
-    rtc::LogMessage::RemoveLogToStream(jni_log_sink.get());
-    jni_log_sink.reset();
-  }
-}
-
 }  // namespace jni
 }  // namespace webrtc