Add a very verbose logging mode for incremental PDF loading Helps to diagnose specific types of behavior we see from the data provider callbacks
Created attachment 393340 [details] Patch v1
Comment on attachment 393340 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=393340&action=review > Source/WebKit/Platform/Logging.h:60 > + M(IncrementalPDFVerbose) \ Just call it IncrementalPDF? > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h:133 > +#ifndef NDEBUG #if !LOG_DISABLED > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h:135 > + size_t addDataCallbackThread() { return ++m_dataCallbackThreadCount; } Is this adding a thread? Or just increasing the callback count? Should it be incrementPendingCallbackCount or something? > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h:385 > +#ifndef NDEBUG #if !LOG_DISABLED > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:636 > +#ifndef NDEBUG #if !LOG_DISABLED > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:646 > + LOG(PDF, "%s", message.utf8().data()); LOG_WITH_STREAM(PDF, stream << message); > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:648 > + LOG(IncrementalPDFVerbose, "%s", message.utf8().data()); LOG_WITH_STREAM(IncrementalPDF, stream << message); > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:690 > + LOG(IncrementalPDFVerbose, "%s", stream.release().utf8().data()); LOG_WITH_STREAM all the pieces at once? > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:698 > +#ifndef NDEBUG #if !LOG_DISABLED > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:699 > + Ref<PDFPlugin> debugPluginRef = *((PDFPlugin*)info); It's a bit nasty of logging changes behavior, like changing object lifetimes. > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:719 > +#ifndef NDEBUG #if !LOG_DISABLED everywhere. > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:742 > + debugPluginRef->pdfLog(stream.release()); LOG_WITH_STREAM directly?
Making most of these changes. Unfortunately with LOG_WITH_STREAM, you (I think?) have to use the TextStream that the macro creates for you, so complicated streams I build up beforehand are a nonstarter.
(In reply to Brady Eidson from comment #3) > Making most of these changes. > > Unfortunately with LOG_WITH_STREAM, you (I think?) have to use the > TextStream that the macro creates for you, so complicated streams I build up > beforehand are a nonstarter. (Note, the reason ::logPDF exists is to log the message to (2) different logging channels, so if somebody builds up a text stream to call into it, LOG_WITH_STREAM won't work and stream.release() is necessary)
Created attachment 393342 [details] PFL
Created attachment 393385 [details] PFL
Created attachment 393450 [details] PFL
Comment on attachment 393450 [details] PFL Clearing flags on attachment: 393450 Committed r258384: <https://trac.webkit.org/changeset/258384>
All reviewed patches have been landed. Closing bug.
<rdar://problem/60405343>
Release build fixed in https://trac.webkit.org/changeset/258389.
(In reply to Ryosuke Niwa from comment #11) > Release build fixed in https://trac.webkit.org/changeset/258389. Wonder why the EWS release builders didn't hit that
(In reply to Brady Eidson from comment #12) > (In reply to Ryosuke Niwa from comment #11) > > Release build fixed in https://trac.webkit.org/changeset/258389. > > Wonder why the EWS release builders didn't hit that (And thanks for the fix!)