Make it possible to observe Logger output. Add a helper class to make logging code less verbose.
Created attachment 319367 [details] Proposed patch.
Attachment 319367 [details] did not pass style-queue: ERROR: Source/WebCore/PAL/pal/Logger.h:59: The parameter name "channel" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 319370 [details] Proposed patch.
Comment on attachment 319370 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=319370&action=review > Source/WebCore/PAL/pal/Logger.h:40 > + template<typename U = T> static typename std::enable_if<std::is_same<U, bool>::value, String>::type toString(bool argument) { return argument ? String("true") : String("false"); } You can use ASCIILiteral for "true" and "false". > Source/WebCore/PAL/pal/Logger.h:45 > + template<typename U = T> static typename std::enable_if<std::is_same<U, AtomicString>::value, String>::type toString(AtomicString argument) { return argument.string(); } > + template<typename U = T> static typename std::enable_if<std::is_same<U, const AtomicString&>::value, String>::type toString(const AtomicString& argument) { return argument.string(); } I wonder if you can use typename std::remove_reference<U>::type in the std::is_same<> check so that you don't have to treat AtomicString and const AtomicString& separately. > Source/WebCore/PAL/pal/Logger.h:47 > + template<typename U = T> static typename std::enable_if<std::is_same<U, const String&>::value, String>::type toString(const String& argument) { return argument; } Ditto. > Source/WebCore/PAL/pal/Logger.h:169 > + static inline void removeObserver(Observer& observer) Looks like it's up to the caller to ensure that removeObserver() is called before an Observer passed to addObserver() is destroyed. Is that the design?
Comment on attachment 319370 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=319370&action=review Thanks! >> Source/WebCore/PAL/pal/Logger.h:40 >> + template<typename U = T> static typename std::enable_if<std::is_same<U, bool>::value, String>::type toString(bool argument) { return argument ? String("true") : String("false"); } > > You can use ASCIILiteral for "true" and "false". Fixed. >> Source/WebCore/PAL/pal/Logger.h:45 >> + template<typename U = T> static typename std::enable_if<std::is_same<U, const AtomicString&>::value, String>::type toString(const AtomicString& argument) { return argument.string(); } > > I wonder if you can use typename std::remove_reference<U>::type in the std::is_same<> check so that you don't have to treat AtomicString and const AtomicString& separately. Great idea, fixed. >> Source/WebCore/PAL/pal/Logger.h:47 >> + template<typename U = T> static typename std::enable_if<std::is_same<U, const String&>::value, String>::type toString(const String& argument) { return argument; } > > Ditto. Ditto. >> Source/WebCore/PAL/pal/Logger.h:169 >> + static inline void removeObserver(Observer& observer) > > Looks like it's up to the caller to ensure that removeObserver() is called before an Observer passed to addObserver() is destroyed. Is that the design? Yes, I wanted to keep this as simple as possible for now. I will reevaluate later when it is used outside of testing.
Created attachment 319382 [details] Patch for landing
Comment on attachment 319382 [details] Patch for landing Clearing flags on attachment: 319382 Committed r221388: <http://trac.webkit.org/changeset/221388>
<rdar://problem/34168545>