113372 2013-03-27 01:34:20 -0700 PDFPlugin: Enable PDFPlugin only if its dependencies exist on the system 2013-03-27 23:23:29 -0700 1 1 1 Unclassified WebKit PDF 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 thorton thorton ap webkit-ews oldest_to_newest 864294 0 thorton 2013-03-27 01:34:20 -0700 Otherwise, fall back to PDFViewController/etc. <rdar://problem/12685301> 864297 1 195243 thorton 2013-03-27 01:37:06 -0700 Created attachment 195243 patch 864299 2 195243 eflews.bot 2013-03-27 01:44:08 -0700 Comment on attachment 195243 patch Attachment 195243 did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17311441 864301 3 195243 thorton 2013-03-27 01:44:52 -0700 Comment on attachment 195243 patch View in context: https://bugs.webkit.org/attachment.cgi?id=195243&action=review > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:42 > +#include "PDFKitImports.h" This needs #if PLATFORM(MAC) too. 864303 4 195243 webkit-ews 2013-03-27 01:46:45 -0700 Comment on attachment 195243 patch Attachment 195243 did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17339071 864583 5 195243 ap 2013-03-27 08:48:40 -0700 Comment on attachment 195243 patch View in context: https://bugs.webkit.org/attachment.cgi?id=195243&action=review >> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:42 >> +#include "PDFKitImports.h" > > This needs #if PLATFORM(MAC) too. (probably in the file itself, not around the include) > Source/WebKit2/WebProcess/WebPage/WebPage.h:616 > + bool pdfPluginEnabled() const; It's not great that this function named as a simple accessor is no longer an accessor. Can we instead perform the check on setting m_pdfPluginEnabled? 864696 6 thorton 2013-03-27 10:58:22 -0700 (In reply to comment #5) > (From update of attachment 195243 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=195243&action=review > > >> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:42 > >> +#include "PDFKitImports.h" > > > > This needs #if PLATFORM(MAC) too. > > (probably in the file itself, not around the include) > > > Source/WebKit2/WebProcess/WebPage/WebPage.h:616 > > + bool pdfPluginEnabled() const; > > It's not great that this function named as a simple accessor is no longer an accessor. Can we instead perform the check on setting m_pdfPluginEnabled? Since checking for PDFLayerController existence requires soft-linking in PDFKit, which we previously only did when it was needed, it seems best to only link it in when we absolutely need the answer (about whether or not we have it), instead of when setting up preferences, no? 864733 7 ap 2013-03-27 11:34:11 -0700 Good point. I'm not sure what a better name would be. 864737 8 thorton 2013-03-27 11:36:28 -0700 (In reply to comment #7) > Good point. I'm not sure what a better name would be. Maybe I should have both pdfPluginEnabled and shouldUsePDFPlugin or something? Only the latter of which does the class check? 865191 9 thorton 2013-03-27 23:23:29 -0700 http://trac.webkit.org/changeset/147067 195243 2013-03-27 01:37:06 -0700 2013-03-27 08:48:40 -0700 patch pdfplugin-on.diff text/plain 2254 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA1ZmEyYzUyLi5mODQ5MTJjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTMtMDMtMjcgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisK KyAgICAgICAgUERGUGx1Z2luOiBFbmFibGUgUERGUGx1Z2luIG9ubHkgaWYgaXRzIGRlcGVuZGVu Y2llcyBleGlzdCBvbiB0aGUgc3lzdGVtCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xMTMzNzIKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzEyNjg1MzAx PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogV2Vi UHJvY2Vzcy9XZWJQYWdlL1dlYlBhZ2UuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJQYWdlOjpw ZGZQbHVnaW5FbmFibGVkKTogSWYgdGhlIFBERlBsdWdpbiBzZXR0aW5nIGlzIGVuYWJsZWQgYnV0 IFBERkxheWVyQ29udHJvbGxlciBpcyBub3QgYXZhaWxhYmxlLCBkb24ndCB1c2UgUERGUGx1Z2lu LgorICAgICAgICAqIFdlYlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmg6CisgICAgICAgIChXZWJQ YWdlKTogTW92ZSBwZGZQbHVnaW5FbmFibGVkKCkncyBpbXBsZW1lbnRhdGlvbiB0byBXZWJQYWdl LmNwcC4KKwogMjAxMy0wMy0yNiAgSGF5YXRvIEl0byAgPGhheWF0b0BjaHJvbWl1bS5vcmc+CiAK ICAgICAgICAgQWxsb3cgU2hhZG93Q29udGVudHMgaW4gSGl0VGVzdHMgYnkgZGVmYXVsdC4KZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmNwcCBi L1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmNwcAppbmRleCBlNGUz ZDI5Li4wYmYwMWZmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBh Z2UvV2ViUGFnZS5jcHAKKysrIGIvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dl YlBhZ2UuY3BwCkBAIC0zOSw2ICszOSw3IEBACiAjaW5jbHVkZSAiTGF5ZXJUcmVlSG9zdC5oIgog I2luY2x1ZGUgIk5ldHNjYXBlUGx1Z2luLmgiCiAjaW5jbHVkZSAiTm90aWZpY2F0aW9uUGVybWlz c2lvblJlcXVlc3RNYW5hZ2VyLmgiCisjaW5jbHVkZSAiUERGS2l0SW1wb3J0cy5oIgogI2luY2x1 ZGUgIlBhZ2VPdmVybGF5LmgiCiAjaW5jbHVkZSAiUGx1Z2luUHJveHkuaCIKICNpbmNsdWRlICJQ bHVnaW5WaWV3LmgiCkBAIC0zNjc2LDYgKzM2NzcsMTMgQEAgdm9pZCBXZWJQYWdlOjpzZXRTY3Jv bGxpbmdQZXJmb3JtYW5jZUxvZ2dpbmdFbmFibGVkKGJvb2wgZW5hYmxlZCkKICAgICBmcmFtZVZp ZXctPnNldFNjcm9sbGluZ1BlcmZvcm1hbmNlTG9nZ2luZ0VuYWJsZWQoZW5hYmxlZCk7CiB9CiAK KyNpZiBQTEFURk9STShNQUMpCitib29sIFdlYlBhZ2U6OnBkZlBsdWdpbkVuYWJsZWQoKSBjb25z dAoreworICAgIHJldHVybiBtX3BkZlBsdWdpbkVuYWJsZWQgJiYgcGRmTGF5ZXJDb250cm9sbGVy Q2xhc3MoKTsKK30KKyNlbmRpZgorCiBib29sIFdlYlBhZ2U6OmNhblBsdWdpbkhhbmRsZVJlc3Bv bnNlKGNvbnN0IFJlc291cmNlUmVzcG9uc2UmIHJlc3BvbnNlKQogewogI2lmIEVOQUJMRShORVRT Q0FQRV9QTFVHSU5fQVBJKQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9X ZWJQYWdlL1dlYlBhZ2UuaCBiL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJQ YWdlLmgKaW5kZXggMzFhOTc2Ny4uOWFhNTRiOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIv V2ViUHJvY2Vzcy9XZWJQYWdlL1dlYlBhZ2UuaAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9j ZXNzL1dlYlBhZ2UvV2ViUGFnZS5oCkBAIC02MTMsNyArNjEzLDcgQEAgcHVibGljOgogICAgIHZv aWQgc2V0U2Nyb2xsaW5nUGVyZm9ybWFuY2VMb2dnaW5nRW5hYmxlZChib29sKTsKIAogI2lmIFBM QVRGT1JNKE1BQykKLSAgICBib29sIHBkZlBsdWdpbkVuYWJsZWQoKSBjb25zdCB7IHJldHVybiBt X3BkZlBsdWdpbkVuYWJsZWQ7IH0KKyAgICBib29sIHBkZlBsdWdpbkVuYWJsZWQoKSBjb25zdDsK ICAgICB2b2lkIHNldFBERlBsdWdpbkVuYWJsZWQoYm9vbCBlbmFibGVkKSB7IG1fcGRmUGx1Z2lu RW5hYmxlZCA9IGVuYWJsZWQ7IH0KICNlbmRpZgogCg==