Summary: | Add a heuristic to determine the “primary” snapshotted plugin | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||||||
Component: | Plug-ins | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | dino, esprehn+autocc, jonlee, ojan.autocc, sam, simon.fraser, webkit-ews, webkit.review.bot | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 145252 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Tim Horton
2013-03-09 17:34:45 PST
Putting it in WK2 because that’ll keep it out of the way for most people... while a heuristic like this doesn’t really belong in WebKit at all - it’s going to require a good bit of effort to make it possible to put it in the client. We plan to eventually expose enough knowledge to do that, but not yet. Created attachment 192362 [details]
patch
Created attachment 192363 [details]
patch
Comment on attachment 192363 [details] patch Attachment 192363 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17009871 Comment on attachment 192363 [details] patch Attachment 192363 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17122184 Created attachment 192364 [details]
patch
Can we use a different #define that PLATFORM(MAC) for this, at the very least to be more descriptive. (In reply to comment #7) > Can we use a different #define that PLATFORM(MAC) for this, at the very least to be more descriptive. Yeah, I was considering that. Currently trying to consolidate all of the #ifs in one file so that that becomes less touch-Platform.h and more add-something-to-the-top-of-WebPage. (In reply to comment #8) > (In reply to comment #7) > > Can we use a different #define that PLATFORM(MAC) for this, at the very least to be more descriptive. > > Yeah, I was considering that. Currently trying to consolidate all of the #ifs in one file so that that becomes less touch-Platform.h and more add-something-to-the-top-of-WebPage. I’m having a hard time telling if that’s a legitimate thing to do. People seem to be using WTF_USE_ for that, which I think is wrong, it should be WTF_ENABLE_, and it’s not done very often. I’ll move it to Platform.h/wherever if someone complains. Created attachment 192367 [details]
patch
Comment on attachment 192367 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=192367&action=review > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3953 > + LayoutUnit contentArea = renderBox->contentWidth() * renderBox->contentHeight(); > + > + if (renderBox->contentWidth() < primarySnapshottedPlugInMinimumWidth || renderBox->contentHeight() < primarySnapshottedPlugInMinimumHeight) > + continue; You can swap these two statements. |