diff options
author | garykac@chromium.org <garykac@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-08-04 03:19:35 +0000 |
---|---|---|
committer | garykac@chromium.org <garykac@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-08-04 03:19:35 +0000 |
commit | 82d105ecf3aadbb90e0adf80f857533967e8f383 (patch) | |
tree | 4dde11c5e525a813ec2dc847e10c1478c7291453 /remoting/client/plugin | |
parent | 05130153c2e1f772955484f89169e505f809ba43 (diff) | |
download | chromium_src-82d105ecf3aadbb90e0adf80f857533967e8f383.zip chromium_src-82d105ecf3aadbb90e0adf80f857533967e8f383.tar.gz chromium_src-82d105ecf3aadbb90e0adf80f857533967e8f383.tar.bz2 |
Modify Chromoting logging to hook into base logging.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/7355011
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@95380 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'remoting/client/plugin')
-rw-r--r-- | remoting/client/plugin/chromoting_instance.cc | 99 | ||||
-rw-r--r-- | remoting/client/plugin/chromoting_instance.h | 21 | ||||
-rw-r--r-- | remoting/client/plugin/chromoting_scriptable_object.cc | 26 | ||||
-rw-r--r-- | remoting/client/plugin/pepper_client_logger.cc | 28 | ||||
-rw-r--r-- | remoting/client/plugin/pepper_client_logger.h | 33 | ||||
-rw-r--r-- | remoting/client/plugin/pepper_view.cc | 19 |
6 files changed, 102 insertions, 124 deletions
diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc index a9c1c1e..6fabb24 100644 --- a/remoting/client/plugin/chromoting_instance.cc +++ b/remoting/client/plugin/chromoting_instance.cc @@ -27,11 +27,12 @@ #include "ppapi/cpp/rect.h" // TODO(wez): Remove this when crbug.com/86353 is complete. #include "ppapi/cpp/private/var_private.h" +#include "remoting/base/task_thread_proxy.h" +#include "remoting/base/util.h" #include "remoting/client/client_config.h" #include "remoting/client/chromoting_client.h" #include "remoting/client/rectangle_update_decoder.h" #include "remoting/client/plugin/chromoting_scriptable_object.h" -#include "remoting/client/plugin/pepper_client_logger.h" #include "remoting/client/plugin/pepper_input_handler.h" #include "remoting/client/plugin/pepper_port_allocator_session.h" #include "remoting/client/plugin/pepper_view.h" @@ -63,23 +64,52 @@ PPP_PolicyUpdate_Dev ChromotingInstance::kPolicyUpdatedInterface = { &ChromotingInstance::PolicyUpdatedThunk, }; +// This flag blocks LOGs to the UI if we're already in the middle of logging +// to the UI. This prevents a potential infinite loop if we encounter an error +// while sending the log message to the UI. +static bool g_logging_to_plugin = false; +static ChromotingInstance* g_logging_instance = NULL; +static logging::LogMessageHandlerFunction g_logging_old_handler = NULL; + const char* ChromotingInstance::kMimeType = "pepper-application/x-chromoting"; ChromotingInstance::ChromotingInstance(PP_Instance pp_instance) : pp::InstancePrivate(pp_instance), initialized_(false), scale_to_fit_(false), - logger_(this), enable_client_nat_traversal_(false), initial_policy_received_(false), task_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) { RequestInputEvents(PP_INPUTEVENT_CLASS_MOUSE | PP_INPUTEVENT_CLASS_WHEEL); RequestFilteringInputEvents(PP_INPUTEVENT_CLASS_KEYBOARD); + + log_proxy_ = new TaskThreadProxy(MessageLoop::current()); + + // Set up log message handler. + // Note that this approach doesn't quite support having multiple instances + // of Chromoting running. In that case, the most recently opened tab will + // grab all the debug log messages, and when any Chromoting tab is closed + // the logging handler will go away. + // Since having multiple Chromoting tabs is not a primary use case, and this + // is just debug logging, we're punting improving debug log support for that + // case. + if (g_logging_old_handler == NULL) + g_logging_old_handler = logging::GetLogMessageHandler(); + logging::SetLogMessageHandler(&LogToUI); + g_logging_instance = this; } ChromotingInstance::~ChromotingInstance() { DCHECK(CurrentlyOnPluginThread()); + // Detach the log proxy so we don't log anything else to the UI. + log_proxy_->Detach(); + + // Remove log message handler. + logging::SetLogMessageHandler(g_logging_old_handler); + g_logging_old_handler = NULL; + g_logging_instance = NULL; + if (client_.get()) { base::WaitableEvent done_event(true, false); client_->Stop(base::Bind(&base::WaitableEvent::Signal, @@ -100,7 +130,7 @@ bool ChromotingInstance::Init(uint32_t argc, CHECK(!initialized_); initialized_ = true; - logger_.VLog(1, "Started ChromotingInstance::Init"); + VLOG(1) << "Started ChromotingInstance::Init"; // Start all the threads. context_.Start(); @@ -126,8 +156,7 @@ void ChromotingInstance::Connect(const ClientConfig& config) { // occurs before the enterprise policy is read. We are guaranteed that the // enterprise policy is pushed at least once, we we delay the connect call. if (!initial_policy_received_) { - logger_.Log(logging::LOG_INFO, - "Delaying connect until initial policy is read."); + LOG(INFO) << "Delaying connect until initial policy is read."; delayed_connect_.reset( task_factory_.NewRunnableMethod(&ChromotingInstance::Connect, config)); @@ -148,7 +177,7 @@ void ChromotingInstance::Connect(const ClientConfig& config) { // If we don't have socket dispatcher for IPC (e.g. P2P API is // disabled), then JingleSessionManager will try to use physical sockets. if (socket_dispatcher) { - logger_.VLog(1, "Creating IpcNetworkManager and IpcPacketSocketFactory."); + VLOG(1) << "Creating IpcNetworkManager and IpcPacketSocketFactory."; network_manager = new IpcNetworkManager(socket_dispatcher); socket_factory = new IpcPacketSocketFactory(socket_dispatcher); } @@ -162,9 +191,9 @@ void ChromotingInstance::Connect(const ClientConfig& config) { client_.reset(new ChromotingClient(config, &context_, host_connection_.get(), view_proxy_, rectangle_decoder_.get(), - input_handler_.get(), &logger_, NULL)); + input_handler_.get(), NULL)); - logger_.Log(logging::LOG_INFO, "Connecting."); + LOG(INFO) << "Connecting."; // Setup the XMPP Proxy. ChromotingScriptableObject* scriptable_object = GetScriptableObject(); @@ -176,7 +205,7 @@ void ChromotingInstance::Connect(const ClientConfig& config) { // Kick off the connection. client_->Start(xmpp_proxy); - logger_.Log(logging::LOG_INFO, "Connection status: Initializing"); + LOG(INFO) << "Connection status: Initializing"; GetScriptableObject()->SetConnectionInfo(STATUS_INITIALIZING, QUALITY_UNKNOWN); } @@ -184,7 +213,7 @@ void ChromotingInstance::Connect(const ClientConfig& config) { void ChromotingInstance::Disconnect() { DCHECK(CurrentlyOnPluginThread()); - logger_.Log(logging::LOG_INFO, "Disconnecting from host."); + LOG(INFO) << "Disconnecting from host."; if (client_.get()) { // TODO(sergeyu): Should we disconnect asynchronously? base::WaitableEvent done_event(true, false); @@ -258,14 +287,14 @@ bool ChromotingInstance::HandleInputEvent(const pp::InputEvent& event) { case PP_INPUTEVENT_TYPE_KEYDOWN: { pp::KeyboardInputEvent key = pp::KeyboardInputEvent(event); - logger_.VLog(3, "PP_INPUTEVENT_TYPE_KEYDOWN key=%d", key.GetKeyCode()); + VLOG(3) << "PP_INPUTEVENT_TYPE_KEYDOWN" << " key=" << key.GetKeyCode(); pih->HandleKeyEvent(true, key); return true; } case PP_INPUTEVENT_TYPE_KEYUP: { pp::KeyboardInputEvent key = pp::KeyboardInputEvent(event); - logger_.VLog(3, "PP_INPUTEVENT_TYPE_KEYUP key=%d", key.GetKeyCode()); + VLOG(3) << "PP_INPUTEVENT_TYPE_KEYUP" << " key=" << key.GetKeyCode(); pih->HandleKeyEvent(false, key); return true; } @@ -291,8 +320,7 @@ ChromotingScriptableObject* ChromotingInstance::GetScriptableObject() { DCHECK(so != NULL); return static_cast<ChromotingScriptableObject*>(so); } - logger_.Log(logging::LOG_ERROR, - "Unable to get ScriptableObject for Chromoting plugin."); + LOG(ERROR) << "Unable to get ScriptableObject for Chromoting plugin."; return NULL; } @@ -300,8 +328,7 @@ void ChromotingInstance::SubmitLoginInfo(const std::string& username, const std::string& password) { if (host_connection_->state() != protocol::ConnectionToHost::STATE_CONNECTED) { - logger_.Log(logging::LOG_INFO, - "Client not connected or already authenticated."); + LOG(INFO) << "Client not connected or already authenticated."; return; } @@ -332,18 +359,32 @@ void ChromotingInstance::SetScaleToFit(bool scale_to_fit) { rectangle_decoder_->RefreshFullFrame(); } -void ChromotingInstance::Log(int severity, const char* format, ...) { - va_list ap; - va_start(ap, format); - logger_.va_Log(severity, format, ap); - va_end(ap); +// static +bool ChromotingInstance::LogToUI(int severity, const char* file, int line, + size_t message_start, + const std::string& str) { + if (g_logging_instance) { + std::string message = remoting::GetTimestampString(); + message += (str.c_str() + message_start); + g_logging_instance->log_proxy_->Call( + base::Bind(&ChromotingInstance::ProcessLogToUI, + base::Unretained(g_logging_instance), message)); + } + + if (g_logging_old_handler) + return (g_logging_old_handler)(severity, file, line, message_start, str); + return false; } -void ChromotingInstance::VLog(int verboselevel, const char* format, ...) { - va_list ap; - va_start(ap, format); - logger_.va_VLog(verboselevel, format, ap); - va_end(ap); +void ChromotingInstance::ProcessLogToUI(const std::string& message) { + if (!g_logging_to_plugin) { + ChromotingScriptableObject* cso = GetScriptableObject(); + if (cso) { + g_logging_to_plugin = true; + cso->LogDebugInfo(message); + g_logging_to_plugin = false; + } + } } pp::Var ChromotingInstance::GetInstanceObject() { @@ -400,13 +441,13 @@ bool ChromotingInstance::IsNatTraversalAllowed( policy_json, true, &error_code, &error_message)); if (!policy.get()) { - logger_.Log(logging::LOG_ERROR, "Error %d parsing policy: %s.", - error_code, error_message.c_str()); + LOG(ERROR) << "Error " << error_code << " parsing policy: " + << error_message << "."; return false; } if (!policy->IsType(base::Value::TYPE_DICTIONARY)) { - logger_.Log(logging::LOG_ERROR, "Policy must be a dictionary"); + LOG(ERROR) << "Policy must be a dictionary"; return false; } diff --git a/remoting/client/plugin/chromoting_instance.h b/remoting/client/plugin/chromoting_instance.h index 787713a..e700893 100644 --- a/remoting/client/plugin/chromoting_instance.h +++ b/remoting/client/plugin/chromoting_instance.h @@ -21,11 +21,8 @@ #include "ppapi/cpp/private/instance_private.h" #include "remoting/client/client_context.h" #include "remoting/client/plugin/chromoting_scriptable_object.h" -#include "remoting/client/plugin/pepper_client_logger.h" #include "remoting/protocol/connection_to_host.h" -class MessageLoop; - namespace base { class Thread; } // namespace base @@ -49,6 +46,7 @@ class JingleThread; class PepperView; class PepperViewProxy; class RectangleUpdateDecoder; +class TaskThreadProxy; struct ClientConfig; @@ -88,9 +86,6 @@ class ChromotingInstance : public pp::InstancePrivate { // Called by ChromotingScriptableObject to set scale-to-fit. void SetScaleToFit(bool scale_to_fit); - void Log(int severity, const char* format, ...); - void VLog(int verboselevel, const char* format, ...); - // Return statistics record by ChromotingClient. // If no connection is currently active then NULL will be returned. ChromotingStats* GetStats(); @@ -99,6 +94,11 @@ class ChromotingInstance : public pp::InstancePrivate { bool DoScaling() const { return scale_to_fit_; } + // A Log Message Handler that is called after each LOG message has been + // processed. This must be of type LogMessageHandlerFunction defined in + // base/logging.h. + static bool LogToUI(int severity, const char* file, int line, + size_t message_start, const std::string& str); private: FRIEND_TEST_ALL_PREFIXES(ChromotingInstanceTest, TestCaseSetup); @@ -109,6 +109,8 @@ class ChromotingInstance : public pp::InstancePrivate { bool IsNatTraversalAllowed(const std::string& policy_json); void HandlePolicyUpdate(const std::string policy_json); + void ProcessLogToUI(const std::string& message); + bool initialized_; ClientContext context_; @@ -118,13 +120,16 @@ class ChromotingInstance : public pp::InstancePrivate { // True if scale to fit is enabled. bool scale_to_fit_; + // A refcounted class to perform thread-switching for logging tasks. + scoped_refptr<TaskThreadProxy> log_proxy_; + // PepperViewProxy is refcounted and used to interface between chromoting // objects and PepperView and perform thread switching. It wraps around // |view_| and receives method calls on chromoting threads. These method // calls are then delegates on the pepper thread. During destruction of // ChromotingInstance we need to detach PepperViewProxy from PepperView since // both ChromotingInstance and PepperView are destroyed and there will be - // outstanding tasks on the pepper message loo. + // outstanding tasks on the pepper message loop. scoped_refptr<PepperViewProxy> view_proxy_; scoped_refptr<RectangleUpdateDecoder> rectangle_decoder_; scoped_ptr<InputHandler> input_handler_; @@ -136,8 +141,6 @@ class ChromotingInstance : public pp::InstancePrivate { // connection. scoped_refptr<PepperXmppProxy> xmpp_proxy_; - PepperClientLogger logger_; - // JavaScript interface to control this instance. // This wraps a ChromotingScriptableObject in a pp::Var. pp::Var instance_object_; diff --git a/remoting/client/plugin/chromoting_scriptable_object.cc b/remoting/client/plugin/chromoting_scriptable_object.cc index 410a4e3..d8a4d6c 100644 --- a/remoting/client/plugin/chromoting_scriptable_object.cc +++ b/remoting/client/plugin/chromoting_scriptable_object.cc @@ -5,7 +5,6 @@ #include "remoting/client/plugin/chromoting_scriptable_object.h" #include "base/logging.h" -#include "base/stringprintf.h" // TODO(wez): Remove this when crbug.com/86353 is complete. #include "ppapi/cpp/private/var_private.h" #include "remoting/base/auth_token_util.h" @@ -229,8 +228,7 @@ void ChromotingScriptableObject::SetConnectionInfo(ConnectionStatus status, int status_index = property_names_[kStatusAttribute]; int quality_index = property_names_[kQualityAttribute]; - LogDebugInfo( - base::StringPrintf("Connection status is updated: %d.", status)); + LOG(INFO) << "Connection status is updated: " << status; if (properties_[status_index].attribute.AsInt() != status || properties_[quality_index].attribute.AsInt() != quality) { @@ -268,8 +266,7 @@ void ChromotingScriptableObject::SetDesktopSize(int width, int height) { SignalDesktopSizeChange(); } - LogDebugInfo(base::StringPrintf("Update desktop size to: %d x %d.", - width, height)); + LOG(INFO) << "Update desktop size to: " << width << " x " << height; } void ChromotingScriptableObject::AddAttribute(const std::string& name, @@ -293,8 +290,7 @@ void ChromotingScriptableObject::SignalConnectionInfoChange() { cb.Call(Var(), &exception); if (!exception.is_undefined()) - LogDebugInfo( - "Exception when invoking connectionInfoUpdate JS callback."); + LOG(ERROR) << "Exception when invoking connectionInfoUpdate JS callback."; } void ChromotingScriptableObject::SignalDesktopSizeChange() { @@ -306,8 +302,8 @@ void ChromotingScriptableObject::SignalDesktopSizeChange() { cb.Call(Var(), &exception); if (!exception.is_undefined()) { - LOG(WARNING) << "Exception when invoking JS callback" - << exception.DebugString(); + LOG(ERROR) << "Exception when invoking JS callback" + << exception.DebugString(); } } @@ -320,7 +316,7 @@ void ChromotingScriptableObject::SignalLoginChallenge() { cb.Call(Var(), &exception); if (!exception.is_undefined()) - LogDebugInfo("Exception when invoking loginChallenge JS callback."); + LOG(ERROR) << "Exception when invoking loginChallenge JS callback."; } void ChromotingScriptableObject::AttachXmppProxy(PepperXmppProxy* xmpp_proxy) { @@ -336,7 +332,7 @@ void ChromotingScriptableObject::SendIq(const std::string& message_xml) { cb.Call(Var(), Var(message_xml), &exception); if (!exception.is_undefined()) - LogDebugInfo("Exception when invoking sendiq JS callback."); + LOG(ERROR) << "Exception when invoking sendiq JS callback."; } Var ChromotingScriptableObject::DoConnect(const std::vector<Var>& args, @@ -379,7 +375,7 @@ Var ChromotingScriptableObject::DoConnect(const std::vector<Var>& args, return Var(); } - LogDebugInfo("Connecting to host."); + LOG(INFO) << "Connecting to host."; VLOG(1) << "client_jid: " << client_jid << ", host_jid: " << host_jid << ", access_code: " << access_code; ClientConfig config; @@ -394,7 +390,7 @@ Var ChromotingScriptableObject::DoConnect(const std::vector<Var>& args, Var ChromotingScriptableObject::DoDisconnect(const std::vector<Var>& args, Var* exception) { - LogDebugInfo("Disconnecting from host."); + LOG(INFO) << "Disconnecting from host."; instance_->Disconnect(); return Var(); @@ -419,7 +415,7 @@ Var ChromotingScriptableObject::DoSubmitLogin(const std::vector<Var>& args, } std::string password = args[1].AsString(); - LogDebugInfo("Submitting login info to host."); + LOG(INFO) << "Submitting login info to host."; instance_->SubmitLoginInfo(username, password); return Var(); } @@ -436,7 +432,7 @@ Var ChromotingScriptableObject::DoSetScaleToFit(const std::vector<Var>& args, return Var(); } - LogDebugInfo("Setting scale-to-fit."); + LOG(INFO) << "Setting scale-to-fit."; instance_->SetScaleToFit(args[0].AsBool()); return Var(); } diff --git a/remoting/client/plugin/pepper_client_logger.cc b/remoting/client/plugin/pepper_client_logger.cc deleted file mode 100644 index 3e13c70..0000000 --- a/remoting/client/plugin/pepper_client_logger.cc +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) 2011 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 "remoting/client/plugin/pepper_client_logger.h" - -//#include <stdarg.h> // va_list - -//#include "base/logging.h" -#include "base/message_loop.h" -//#include "base/stringprintf.h" -#include "remoting/client/plugin/chromoting_instance.h" - -namespace remoting { - -PepperClientLogger::PepperClientLogger(ChromotingInstance* instance) - : instance_(instance) { - SetThread(MessageLoop::current()); -} - -PepperClientLogger::~PepperClientLogger() { -} - -void PepperClientLogger::LogToUI(const std::string& message) { - instance_->GetScriptableObject()->LogDebugInfo(message); -} - -} // namespace remoting diff --git a/remoting/client/plugin/pepper_client_logger.h b/remoting/client/plugin/pepper_client_logger.h deleted file mode 100644 index badf4d2..0000000 --- a/remoting/client/plugin/pepper_client_logger.h +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2011 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. - -#ifndef REMOTING_CLIENT_PLUGIN_PEPPER_CLIENT_LOGGER_H_ -#define REMOTING_CLIENT_PLUGIN_PEPPER_CLIENT_LOGGER_H_ - -#include "remoting/base/logger.h" - -#include "base/task.h" - -class MessageLoop; - -namespace remoting { - -class ChromotingInstance; - -class PepperClientLogger : public Logger { - public: - explicit PepperClientLogger(ChromotingInstance* instance); - virtual ~PepperClientLogger(); - - virtual void LogToUI(const std::string& message); - - private: - ChromotingInstance* instance_; - - DISALLOW_COPY_AND_ASSIGN(PepperClientLogger); -}; - -} // namespace remoting - -#endif // REMOTING_CLIENT_PLUGIN_PEPPER_CLIENT_LOGGER_H_ diff --git a/remoting/client/plugin/pepper_view.cc b/remoting/client/plugin/pepper_view.cc index 72cae5b..f85ca71 100644 --- a/remoting/client/plugin/pepper_view.cc +++ b/remoting/client/plugin/pepper_view.cc @@ -48,16 +48,15 @@ void PepperView::Paint() { TraceContext::tracer()->PrintString("Start Paint."); if (is_static_fill_) { - instance_->Log(logging::LOG_INFO, - "Static filling %08x", static_fill_color_); + LOG(INFO) << "Static filling " << static_fill_color_; pp::ImageData image(instance_, pp::ImageData::GetNativeImageDataFormat(), pp::Size(graphics2d_.size().width(), graphics2d_.size().height()), false); if (image.is_null()) { - instance_->Log(logging::LOG_ERROR, - "Unable to allocate image of size: %dx%d", - graphics2d_.size().width(), graphics2d_.size().height()); + LOG(ERROR) << "Unable to allocate image of size: " + << graphics2d_.size().width() << " x " + << graphics2d_.size().height(); return; } @@ -100,7 +99,7 @@ void PepperView::PaintFrame(media::VideoFrame* frame, UpdatedRects* rects) { SetHostSize(gfx::Size(frame->width(), frame->height())); if (!backing_store_.get() || backing_store_->is_null()) { - instance_->Log(logging::LOG_ERROR, "Backing store is not available."); + LOG(ERROR) << "Backing store is not available."; return; } @@ -255,7 +254,7 @@ bool PepperView::SetPluginSize(const gfx::Size& plugin_size) { graphics2d_ = pp::Graphics2D(instance_, pp_size, true); if (!instance_->BindGraphics(graphics2d_)) { - instance_->Log(logging::LOG_ERROR, "Couldn't bind the device context."); + LOG(ERROR) << "Couldn't bind the device context."; return false; } @@ -265,8 +264,8 @@ bool PepperView::SetPluginSize(const gfx::Size& plugin_size) { // Allocate the backing store to save the desktop image. if ((backing_store_.get() == NULL) || (backing_store_->size() != pp_size)) { - instance_->Log(logging::LOG_INFO, "Allocate backing store: %d x %d", - plugin_size.width(), plugin_size.height()); + LOG(INFO) << "Allocate backing store: " + << plugin_size.width() << " x " << plugin_size.height(); backing_store_.reset( new pp::ImageData(instance_, pp::ImageData::GetNativeImageDataFormat(), pp_size, false)); @@ -314,7 +313,7 @@ void PepperView::ReleaseFrame(media::VideoFrame* frame) { DCHECK(CurrentlyOnPluginThread()); if (frame) { - instance_->Log(logging::LOG_WARNING, "Frame released."); + LOG(WARNING) << "Frame released."; frame->Release(); } } |