Display the proxy PAC javascript errors in the NetLog.

BUG=47226
TEST=Configure chrome with a PAC script that throws errors. Load a URL and should see the javascript error displayed on about:net-internals.
Review URL: http://codereview.chromium.org/2978001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@52253 0039d316-1c4b-4281-b951-d872f2087c98

net/base/forwarding_net_log.cc

@@ -0,0 +1,101 @@
+// Copyright (c) 2010 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 "net/base/forwarding_net_log.h"
+
+#include "base/lock.h"
+#include "base/logging.h"
+#include "base/message_loop.h"
+
+namespace net {
+
+// Reference-counted wrapper, so we can use PostThread and it can safely
+// outlive the parent ForwardingNetLog.
+class ForwardingNetLog::Core
+    : public base::RefCountedThreadSafe<ForwardingNetLog::Core> {
+ public:
+  Core(NetLog* impl, MessageLoop* loop) : impl_(impl), loop_(loop) {
+    DCHECK(impl);
+    DCHECK(loop);
+  }
+
+  // Called once the parent ForwardingNetLog is being destroyed. It
+  // is invalid to access |loop_| and |impl_| afterwards.
+  void Orphan() {
+    AutoLock l(lock_);
+    loop_ = NULL;
+    impl_ = NULL;
+  }
+
+  void AddEntry(EventType type,
+                const base::TimeTicks& time,
+                const Source& source,
+                EventPhase phase,
+                EventParameters* params) {
+    AutoLock l(lock_);
+    if (!loop_)
+      return;  // Was orphaned.
+
+    loop_->PostTask(
+        FROM_HERE,
+        NewRunnableMethod(
+            this, &Core::AddEntryOnLoop, type, time, source, phase,
+            scoped_refptr<EventParameters>(params)));
+  }
+
+ private:
+  void AddEntryOnLoop(EventType type,
+                      const base::TimeTicks& time,
+                      const Source& source,
+                      EventPhase phase,
+                      scoped_refptr<EventParameters> params) {
+    AutoLock l(lock_);
+    if (!loop_)
+      return;  // Was orphaned.
+
+    DCHECK_EQ(MessageLoop::current(), loop_);
+
+    // TODO(eroman): This shouldn't be necessary. See crbug.com/48806.
+    NetLog::Source effective_source = source;
+    if (effective_source.id == NetLog::Source::kInvalidId)
+      effective_source.id = impl_->NextID();
+
+    impl_->AddEntry(type, time, effective_source, phase, params);
+  }
+
+  Lock lock_;
+  NetLog* impl_;
+  MessageLoop* loop_;
+};
+
+ForwardingNetLog::ForwardingNetLog(NetLog* impl, MessageLoop* loop)
+    : core_(new Core(impl, loop)) {
+}
+
+ForwardingNetLog::~ForwardingNetLog() {
+  core_->Orphan();
+}
+
+void ForwardingNetLog::AddEntry(EventType type,
+                                const base::TimeTicks& time,
+                                const Source& source,
+                                EventPhase phase,
+                                EventParameters* params) {
+  core_->AddEntry(type, time, source, phase, params);
+}
+
+uint32 ForwardingNetLog::NextID() {
+  // Can't forward a synchronous API.
+  CHECK(false) << "Not supported";
+  return 0;
+}
+
+bool ForwardingNetLog::HasListener() const {
+  // Can't forward a synchronous API.
+  CHECK(false) << "Not supported";
+  return false;
+}
+
+}  // namespace net
+

net/base/forwarding_net_log.h

@@ -0,0 +1,53 @@
+// Copyright (c) 2010 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 NET_BASE_FORWARDING_NET_LOG_H_
+#define NET_BASE_FORWARDING_NET_LOG_H_
+
+#include "base/basictypes.h"
+#include "net/base/net_log.h"
+
+class MessageLoop;
+
+namespace net {
+
+// ForwardingNetLog is a wrapper that can be called on any thread, and will
+// forward any calls to NetLog::AddEntry() over to |impl| on the specified
+// message loop.
+//
+// This allows using a non-threadsafe NetLog implementation from another
+// thread.
+//
+// TODO(eroman): Explore making NetLog threadsafe and obviating the need
+//               for this class.
+class ForwardingNetLog : public NetLog {
+ public:
+  // Both |impl| and |loop| must outlive the lifetime of this instance.
+  // |impl| will be operated only from |loop|.
+  ForwardingNetLog(NetLog* impl, MessageLoop* loop);
+
+  // On destruction any outstanding call to AddEntry() which didn't make
+  // it to |loop| yet will be cancelled.
+  ~ForwardingNetLog();
+
+  // NetLog methods:
+  virtual void AddEntry(EventType type,
+                        const base::TimeTicks& time,
+                        const Source& source,
+                        EventPhase phase,
+                        EventParameters* params);
+  virtual uint32 NextID();
+  virtual bool HasListener() const;
+
+ private:
+  class Core;
+  scoped_refptr<Core> core_;
+
+  DISALLOW_COPY_AND_ASSIGN(ForwardingNetLog);
+};
+
+}  // namespace net
+
+#endif  // NET_BASE_FORWARDING_NET_LOG_H_
+

net/base/forwarding_net_log_unittest.cc

@@ -0,0 +1,84 @@
+// Copyright (c) 2010 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 "net/base/forwarding_net_log.h"
+
+#include "base/message_loop.h"
+#include "net/base/capturing_net_log.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+// Test forwarding a call to AddEntry() to another implementation, operating
+// on this same message loop.
+TEST(ForwardingNetLogTest, Basic) {
+  // Create a forwarding NetLog that sends messages to this same thread.
+  CapturingNetLog log(CapturingNetLog::kUnbounded);
+  ForwardingNetLog forwarding(&log, MessageLoop::current());
+
+  EXPECT_EQ(0u, log.entries().size());
+
+  NetLogStringParameter* params = new NetLogStringParameter("xxx", "yyy");
+
+  forwarding.AddEntry(
+      NetLog::TYPE_PAC_JAVASCRIPT_ALERT,
+      base::TimeTicks(),
+      NetLog::Source(),
+      NetLog::PHASE_NONE,
+      params);
+
+  // Should still be empty, since we posted an async task.
+  EXPECT_EQ(0u, log.entries().size());
+
+  MessageLoop::current()->RunAllPending();
+
+  // After draining the message loop, we should now have executed the task
+  // and hence emitted the log entry.
+  ASSERT_EQ(1u, log.entries().size());
+
+  // Check that the forwarded call contained received all the right inputs.
+  EXPECT_EQ(NetLog::TYPE_PAC_JAVASCRIPT_ALERT, log.entries()[0].type);
+  EXPECT_EQ(NetLog::SOURCE_NONE, log.entries()[0].source.type);
+  EXPECT_EQ(NetLog::PHASE_NONE, log.entries()[0].phase);
+  EXPECT_EQ(params, log.entries()[0].extra_parameters.get());
+
+  // Check that the parameters is still referenced. (if the reference was
+  // lost then this will be a memory error and probaby crash).
+  EXPECT_EQ("yyy", params->value());
+}
+
+// Test forwarding a call to AddEntry() to another implementation that runs
+// on the same message loop. However destroy the forwarder before the posted
+// task has a chance to run.
+TEST(ForwardingNetLogTest, Orphan) {
+  // Create a forwarding NetLog that sends messages to this same thread.
+  CapturingNetLog log(CapturingNetLog::kUnbounded);
+  {
+    ForwardingNetLog forwarding(&log, MessageLoop::current());
+    EXPECT_EQ(0u, log.entries().size());
+
+    forwarding.AddEntry(
+        NetLog::TYPE_PAC_JAVASCRIPT_ALERT,
+        base::TimeTicks(),
+        NetLog::Source(),
+        NetLog::PHASE_NONE,
+        NULL);
+
+    // Should still be empty, since we posted an async task.
+    EXPECT_EQ(0u, log.entries().size());
+  }
+
+  // At this point the ForwardingNetLog is deleted. However it had already
+  // posted a task to the message loop. Once we drain the message loop, we
+  // verify that the task didn't actually try to emit to the NetLog.
+  MessageLoop::current()->RunAllPending();
+  EXPECT_EQ(0u, log.entries().size());
+}
+
+}  // namespace
+
+}  // namespace net
+

net/base/net_log.h

@@ -101,6 +101,7 @@ class NetLog {
   //  |params| - Optional (may be NULL) parameters for this event.
   //             The specific subclass of EventParameters is defined
   //             by the contract for events of this |type|.
+  //             TODO(eroman): Take a scoped_refptr<> instead.
   virtual void AddEntry(EventType type,
                         const base::TimeTicks& time,
                         const Source& source,

net/base/net_log_event_type_list.h

@@ -99,16 +99,32 @@ EVENT_TYPE(PROXY_SERVICE_RESOLVED_PROXY_LIST)
 // ------------------------------------------------------------------------
 
 // Measures the time taken to execute the "myIpAddress()" javascript binding.
-EVENT_TYPE(PROXY_RESOLVER_V8_MY_IP_ADDRESS)
+EVENT_TYPE(PAC_JAVASCRIPT_MY_IP_ADDRESS)
 
 // Measures the time taken to execute the "myIpAddressEx()" javascript binding.
-EVENT_TYPE(PROXY_RESOLVER_V8_MY_IP_ADDRESS_EX)
+EVENT_TYPE(PAC_JAVASCRIPT_MY_IP_ADDRESS_EX)
 
 // Measures the time taken to execute the "dnsResolve()" javascript binding.
-EVENT_TYPE(PROXY_RESOLVER_V8_DNS_RESOLVE)
+EVENT_TYPE(PAC_JAVASCRIPT_DNS_RESOLVE)
 
 // Measures the time taken to execute the "dnsResolveEx()" javascript binding.
-EVENT_TYPE(PROXY_RESOLVER_V8_DNS_RESOLVE_EX)
+EVENT_TYPE(PAC_JAVASCRIPT_DNS_RESOLVE_EX)
+
+// This event is emitted when a javascript error has been triggered by a
+// PAC script. It contains the following event parameters:
+//   {
+//      "line_number": <The line number in the PAC script
+//                      (or -1 if not applicable)>,
+//      "message": <The error message>
+//   }
+EVENT_TYPE(PAC_JAVASCRIPT_ERROR)
+
+// This event is emitted when a PAC script called alert(). It contains the
+// following event parameters:
+//   {
+//      "message": <The string of the alert>
+//   }
+EVENT_TYPE(PAC_JAVASCRIPT_ALERT)
 
 // Measures the time that a proxy resolve request was stalled waiting for a
 // proxy resolver thread to free-up.

net/net.gyp

@@ -62,6 +62,8 @@
         'base/file_stream_win.cc',
         'base/filter.cc',
         'base/filter.h',
+        'base/forwarding_net_log.cc',
+        'base/forwarding_net_log.h',
         'base/gzip_filter.cc',
         'base/gzip_filter.h',
         'base/gzip_header.cc',
@@ -654,6 +656,7 @@
         'base/file_stream_unittest.cc',
         'base/filter_unittest.cc',
         'base/filter_unittest.h',
+        'base/forwarding_net_log_unittest.cc',
         'base/gzip_filter_unittest.cc',
         'base/host_cache_unittest.cc',
         'base/host_mapping_rules_unittest.cc',

net/proxy/multi_threaded_proxy_resolver.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 

net/proxy/multi_threaded_proxy_resolver_unittest.cc

@@ -46,7 +46,7 @@ class MockProxyResolver : public ProxyResolver {
     EXPECT_TRUE(request == NULL);
 
     // Write something into |net_log| (doesn't really have any meaning.)
-    net_log.BeginEvent(NetLog::TYPE_PROXY_RESOLVER_V8_DNS_RESOLVE, NULL);
+    net_log.BeginEvent(NetLog::TYPE_PAC_JAVASCRIPT_DNS_RESOLVE, NULL);
 
     results->UseNamedProxy(query_url.host());