Implement WebProcess::platformSetCacheModel() for Qt and expose CacheModel API so that host apps, such as browser, can set it properly.
Created attachment 124551 [details] First patch. First patch for implementation of the patch model. It adds the implementation for Qt port of WebKit2 for Mac and Linux systems. Next patches to be made are the implementation for Qt/WK2 on Windows, the implementation of hybrid memory and disk cache and the implementation for the WK2/Qt Quick API.
Attachment 124551 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:85: Should have a space between // and comment [whitespace/comments] [4] Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:95: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:100: Should have a space between // and comment [whitespace/comments] [4] Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:131: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 4 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 124551 [details] First patch. Found style issues that I'll fix first. Sorry for that.
Created attachment 124553 [details] First patch (updated) Fixed style issues.
Comment on attachment 124553 [details] First patch (updated) View in context: https://bugs.webkit.org/attachment.cgi?id=124553&action=review Hello Michael, I have some suggestions and a question. :-) > Source/WebKit2/UIProcess/qt/WebContextQt.cpp:60 > +static QString defaultDiskCacheDirectory() I think this could be String (from WTF) instead of QString. Both functions you call here in WebCore return String and this function is used to fill a String too. > Source/WebKit2/UIProcess/qt/WebContextQt.cpp:65 > + if (!s_defaulDiskCacheDirectory.isEmpty()) > + return s_defaulDiskCacheDirectory; In case of lazy initialization I tend to write down the initialization block here, and after the conditional, one return. Just a minor suggestion, feel free to ignore. > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:109 > + // The Mac port of WebKit2 uses a fudge factor of 1000 here to account for misalignment, howver, Typo: "however". > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:112 > + qint64 freeMemory = memorySize() / 1024 / 1023; If I understood correctly memorySize() calculates the full physical memory size, not only the free memory. I think this is confusing. Also freeMemory must be understood in MB? If so the conversion is confusing to me: it will give us more memory than the physical, or am I reading something wrong?
(In reply to comment #5) > (From update of attachment 124553 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=124553&action=review > > Hello Michael, I have some suggestions and a question. :-) Thanks for the suggestions :). > > > Source/WebKit2/UIProcess/qt/WebContextQt.cpp:60 > > +static QString defaultDiskCacheDirectory() > > I think this could be String (from WTF) instead of QString. Both functions you call here in WebCore return String and this function is used to fill a String too. Okay, true - I just did this to stay inline with the other methods in WebContextQt.cpp, they also use QString. > > > Source/WebKit2/UIProcess/qt/WebContextQt.cpp:65 > > + if (!s_defaulDiskCacheDirectory.isEmpty()) > > + return s_defaulDiskCacheDirectory; > > In case of lazy initialization I tend to write down the initialization block here, and after the conditional, one return. Just a minor suggestion, feel free to ignore. Yeah, actually, I would have done the same, but the other methods also do it this way and I think the WebKit style guide says that one should do early returns. > > > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:109 > > + // The Mac port of WebKit2 uses a fudge factor of 1000 here to account for misalignment, howver, > > Typo: "however". Nice catch, will fix :) > > > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:112 > > + qint64 freeMemory = memorySize() / 1024 / 1023; > > If I understood correctly memorySize() calculates the full physical memory size, not only the free memory. I think this is confusing. Yeah, I forgot to rename since I misinterpreted what was needed for the calcualation. I'll rename it. > > Also freeMemory must be understood in MB? If so the conversion is confusing to me: it will give us more memory than the physical, or am I reading something wrong? The fudge factor (inspired from the Mac version) is basically there to account the fact that memory might not necessarily aligned to a full Megabyte. This is probably more important for the free disk space than for the memory, but we can also remove it completely.
Comment on attachment 124553 [details] First patch (updated) View in context: https://bugs.webkit.org/attachment.cgi?id=124553&action=review How does all of this tie in with the MemoryPressure (or what is its name) class in WebKit? > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:65 > + // Most of this part was copied from the Mac port (see Source/WebKit2/WebProcess/mac/WebProcessMac.mm for details). > + static host_basic_info_data_t hostInfo; Can't it be shared instead?
(In reply to comment #7) > (From update of attachment 124553 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=124553&action=review > > How does all of this tie in with the MemoryPressure (or what is its name) class in WebKit? It calculates the limits that the MemoryPressureHandler and the MemoryCache will have for operation. > > > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:65 > > + // Most of this part was copied from the Mac port (see Source/WebKit2/WebProcess/mac/WebProcessMac.mm for details). > > + static host_basic_info_data_t hostInfo; > > Can't it be shared instead? Sure, we could make the Mac part a shared private method in the WebProcess if that's what you meant.
Created attachment 124699 [details] First patch (updated with suggestions). Updated the patch with suggestions from Caio Marcelo and Kenneth.
Attachment 124699 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource First, rewinding head to replay your work on top of it... Applying: Fix compilation errors on build-webkit --debug --no-workers on mac. Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging LayoutTests/ChangeLog CONFLICT (content): Merge conflict in LayoutTests/ChangeLog Auto-merging LayoutTests/platform/qt/Skipped CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped Auto-merging Source/WebCore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Failed to merge in the changes. Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac. When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #10) > Attachment 124699 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 > > Updating OpenSource > First, rewinding head to replay your work on top of it... > Applying: Fix compilation errors on build-webkit --debug --no-workers on mac. > Using index info to reconstruct a base tree... > Falling back to patching base and 3-way merge... > Auto-merging LayoutTests/ChangeLog > CONFLICT (content): Merge conflict in LayoutTests/ChangeLog > Auto-merging LayoutTests/platform/qt/Skipped > CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped > Auto-merging Source/WebCore/ChangeLog > CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog > Failed to merge in the changes. > Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac. > > When you have resolved this problem run "git rebase --continue". > If you would prefer to skip this patch, instead run "git rebase --skip". > To restore the original branch and stop rebasing run "git rebase --abort". > > rebase refs/remotes/origin/master: command returned error: 1 > > Died at Tools/Scripts/update-webkit line 164. > > > If any of these errors are false positives, please file a bug against check-webkit-style. Seems as if either the style-commit-queue is hosed or it has problems with git patches. Anyway, none of the changes I made touched any if these files...
Comment on attachment 124699 [details] First patch (updated with suggestions). View in context: https://bugs.webkit.org/attachment.cgi?id=124699&action=review > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:69 > + long pageSize = sysconf(_SC_PAGESIZE); > + long numberOfPages = sysconf(_SC_PHYS_PAGES); > + > + if (pageSize > 0 && numberOfPages > 0) > + s_physicalMemorySize = static_cast<qint64>(pageSize) * static_cast<qint64>(numberOfPages); why not turn this into a memorySize method for UNIX platforms? Would be great to have a class for all of these, so they can be shared better. like PlatformInfoMac, PlatformInfoUNIX etc > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:73 > + // FIXME: Find a correct implementation of this for QtWebKit on Windows. Currently this is not just windows... could be QNX for instance.
Created attachment 125140 [details] Updated patch. Added the Platform class plus implementation for unix/posix, windows and mac.
Created attachment 125310 [details] Updated patch. Fixed build problem for Qt port on Mac.
Comment on attachment 125310 [details] Updated patch. View in context: https://bugs.webkit.org/attachment.cgi?id=125310&action=review > Source/WebCore/platform/PlatformInfo.cpp:31 > +PlatformInfo* platformInfo() I would prefer something like PlatformInfo::instance() but i am not sure of the pattern used in webkit > Source/WebCore/platform/PlatformInfo.cpp:33 > + static PlatformInfo* staticInfo = new PlatformInfo; I would prefer the name 'instance' > Source/WebCore/platform/mac/PlatformInfoMac.cpp:36 > +uint64_t PlatformInfo::memorySize() so is this available memory, total memory, is it in mega bytes, bytes etc? I think the name could be better > Source/WebCore/platform/qt/FileSystemQt.cpp:194 > +uint64_t getVolumeFreeSizeForPath(const char* path) Size? in bytes? mega bytes? availableBytesOnVolumeAtPath ? :-)
(In reply to comment #15) > (From update of attachment 125310 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=125310&action=review > > > Source/WebCore/platform/PlatformInfo.cpp:31 > > +PlatformInfo* platformInfo() > > I would prefer something like PlatformInfo::instance() but i am not sure of the pattern used in webkit > > > Source/WebCore/platform/PlatformInfo.cpp:33 > > + static PlatformInfo* staticInfo = new PlatformInfo; > > I would prefer the name 'instance' > > > Source/WebCore/platform/mac/PlatformInfoMac.cpp:36 > > +uint64_t PlatformInfo::memorySize() > > so is this available memory, total memory, is it in mega bytes, bytes etc? I think the name could be better I agree - I just copied the name from the Mac and GTK WebProcess implementation for consistency / for possible use by those ports in the future. Should we stay consistent with those or should we just change the name? > > > Source/WebCore/platform/qt/FileSystemQt.cpp:194 > > +uint64_t getVolumeFreeSizeForPath(const char* path) > > Size? in bytes? mega bytes? > > availableBytesOnVolumeAtPath ? :-) This is a method that was already implemented on the GTK port. Should we change the name and also refactor the GTK code?
Created attachment 125626 [details] Patch
Created attachment 125627 [details] Patch
Created attachment 125629 [details] Patch
Comment on attachment 125629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125629&action=review > Source/WebKit2/UIProcess/qt/WebContextQt.cpp:62 > + static String s_defaulDiskCacheDirectory; spelling error > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:74 > + unsigned cacheTotalCapacity = 0; > + unsigned cacheMinDeadCapacity = 0; > + unsigned cacheMaxDeadCapacity = 0; > + double deadDecodedDataDeletionInterval = 0; > + unsigned pageCacheCapacity = 0; > + unsigned long urlCacheMemoryCapacity = 0; > + unsigned long urlCacheDiskCapacity = 0; why do you need to initialize these to 0? Doesn't the calculateCacheSizes aways set the values? > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:104 > + QNetworkDiskCache* diskCache = new QNetworkDiskCache(); does the network access manager take over ownership?
(In reply to comment #20) > (From update of attachment 125629 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=125629&action=review > > > Source/WebKit2/UIProcess/qt/WebContextQt.cpp:62 > > + static String s_defaulDiskCacheDirectory; > > spelling error > > > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:74 > > + unsigned cacheTotalCapacity = 0; > > + unsigned cacheMinDeadCapacity = 0; > > + unsigned cacheMaxDeadCapacity = 0; > > + double deadDecodedDataDeletionInterval = 0; > > + unsigned pageCacheCapacity = 0; > > + unsigned long urlCacheMemoryCapacity = 0; > > + unsigned long urlCacheDiskCapacity = 0; > > why do you need to initialize these to 0? Doesn't the calculateCacheSizes aways set the values? > > > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:104 > > + QNetworkDiskCache* diskCache = new QNetworkDiskCache(); > > does the network access manager take over ownership? (In reply to comment #20) > (From update of attachment 125629 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=125629&action=review > > > Source/WebKit2/UIProcess/qt/WebContextQt.cpp:62 > > + static String s_defaulDiskCacheDirectory; > > spelling error Good catch :) > > > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:74 > > + unsigned cacheTotalCapacity = 0; > > + unsigned cacheMinDeadCapacity = 0; > > + unsigned cacheMaxDeadCapacity = 0; > > + double deadDecodedDataDeletionInterval = 0; > > + unsigned pageCacheCapacity = 0; > > + unsigned long urlCacheMemoryCapacity = 0; > > + unsigned long urlCacheDiskCapacity = 0; > > why do you need to initialize these to 0? Doesn't the calculateCacheSizes aways set the values? It's basically what all the other ports do. Anyway, I checked and it seems there is one case where cacheTotalCapacity does not get set (that happens if the physical memory of the device is smaller than 512 MB) and subsequently, cacheMinDeadCapacity and CacheMaxDeadCapacity might not be set correctly. We could fix this and move the initialization to 0 to WebProcess::calculateCacheSizes, but I don't think the initialization's here hurt as they will probably get optimised out anyway. I agree that initializing here is unnecessary if everything gets reset anyway, but is it against some Webkit guideline to do this? > > > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:104 > > + QNetworkDiskCache* diskCache = new QNetworkDiskCache(); > > does the network access manager take over ownership? Yes, it does.
Created attachment 125640 [details] Patch
Created attachment 125660 [details] Patch
Comment on attachment 125660 [details] Patch OK, lets go with this for now.
Comment on attachment 125660 [details] Patch I'll follow up with the PlatformInfo patch and cc Anders or Kling.
Comment on attachment 125660 [details] Patch Clearing flags on attachment: 125660 Committed r106920: <http://trac.webkit.org/changeset/106920>
All reviewed patches have been landed. Closing bug.
(In reply to comment #26) > (From update of attachment 125660 [details]) > Clearing flags on attachment: 125660 > > Committed r106920: <http://trac.webkit.org/changeset/106920> Reopen, because it broke the Qt Windows build: ../../../../Source/WebCore/platform/qt/FileSystemQt.cpp:42:25: fatal error: sys/statvfs.h: No such file or directory Could you fix it?
Created attachment 125809 [details] Patch
Comment on attachment 125809 [details] Patch Invalid.
Created attachment 125810 [details] Patch
Comment on attachment 125810 [details] Patch rs=me, I'll land it manually.
Comment on attachment 125810 [details] Patch Clearing flags on attachment: 125810 Committed r106924: <http://trac.webkit.org/changeset/106924>
Reopen, because win build is still broken: ../../../../Source/WebCore/platform/qt/FileSystemQt.cpp: In function 'uint64_t WebCore::getVolumeFreeSizeForPath(const char*)': ../../../../Source/WebCore/platform/qt/FileSystemQt.cpp:203:33: error: expected '(' before 'freeBytesToCaller' ../../../../Source/WebCore/platform/qt/FileSystemQt.cpp:203:59: error: expected ')' before ';' token
And it broke many tests on WK2: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r106920%20%2819804%29/results.html (fails before your change: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r106919%20%2819803%29/results.html)
Created attachment 125821 [details] Patch
Could you guys land buildfixes manually instad of waiting hours for commit queue?
Comment on attachment 125821 [details] Patch Rejecting attachment 125821 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ebKit/chromium/third_party/yasm/source/patched-yasm --revision 73761 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 46>At revision 73761. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/11436305
Comment on attachment 125821 [details] Patch Landed in http://trac.webkit.org/changeset/106933
Leave it open, because WK2 tests are still broken.
Created attachment 125843 [details] Patch
Comment on attachment 125843 [details] Patch bbandix was the faster - http://trac.webkit.org/changeset/106939
I skipped the failing tests until fix - http://trac.webkit.org/changeset/106954
(In reply to comment #44) > I skipped the failing tests until fix - http://trac.webkit.org/changeset/106954 WK2 fails can be found here: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r106951%20%2819821%29/results.html
(In reply to comment #45) > (In reply to comment #44) > > I skipped the failing tests until fix - http://trac.webkit.org/changeset/106954 > > WK2 fails can be found here: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r106951%20%2819821%29/results.html Michael, the fact that fast/harness/use-page-cache.html is not passing maybe indicates that the page cache capacity is not enough. One idea is to check which value those calculations are giving you for page capacity. Also, enforce page capacity 20 to see if the test pass.
(In reply to comment #46) > Michael, the fact that fast/harness/use-page-cache.html is not passing maybe indicates that the page cache capacity is not enough. One idea is to check which value those calculations are giving you for page capacity. Also, enforce page capacity 20 to see if the test pass. Magic number 20 was the old default :-)
(In reply to comment #46) > (In reply to comment #45) > > (In reply to comment #44) > > > I skipped the failing tests until fix - http://trac.webkit.org/changeset/106954 > > > > WK2 fails can be found here: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r106951%20%2819821%29/results.html > > Michael, the fact that fast/harness/use-page-cache.html is not passing maybe indicates that the page cache capacity is not enough. One idea is to check which value those calculations are giving you for page capacity. Also, enforce page capacity 20 to see if the test pass. Yeah, I also found that this was the reason - the default CacheModel results in pageCacheCapacity being 0. The gtk port sets this to another value, while the Mac port does not seem to reset it from the default value. I have a patch built & running the test at the office, but had to leave before it finished. I'll post the patch tomorrow if it fixes the issues...
Created attachment 126072 [details] Patch
Comment on attachment 126072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126072&action=review > Source/WebKit2/ChangeLog:9 > + (WebKit::WebContext::WebContext): Set default cacheModel for QT platform to Qt > Source/WebKit2/UIProcess/WebContext.cpp:129 > +#if PLATFORM(QT) > + , m_cacheModel(CacheModelPrimaryWebBrowser) > +#else > , m_cacheModel(CacheModelDocumentViewer) > +#endif We might want an experimental setting for this with the document viewer being the default
(In reply to comment #50) > (From update of attachment 126072 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126072&action=review > > > Source/WebKit2/ChangeLog:9 > > + (WebKit::WebContext::WebContext): Set default cacheModel for QT platform to > > Qt Will fix. So "Qt" is the preferred way of writing? I'm asking because I've seen mixes of Qt, QT and qt being used in logs... > > > Source/WebKit2/UIProcess/WebContext.cpp:129 > > +#if PLATFORM(QT) > > + , m_cacheModel(CacheModelPrimaryWebBrowser) > > +#else > > , m_cacheModel(CacheModelDocumentViewer) > > +#endif > > We might want an experimental setting for this with the document viewer being the default As mentioned in #qtwebkit, the experimental setting will be part of the cacheModel API implementation for Qt-WK2, which I'll post soon. However, I think it's important that we unskip those layout tests for the time being.
It has always been Qt. QT == QuickTime
Created attachment 126079 [details] Patch
Comment on attachment 126079 [details] Patch Corrected ChangeLogs
Comment on attachment 126079 [details] Patch Clearing flags on attachment: 126079 Committed r107090: <http://trac.webkit.org/changeset/107090>
Reopen, because the bug is still valid. You broke 19 tests now: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r107090%20%2819898%29/results.html Please run layout tests before commiting and _watch_ the bots after landing.
I think it's time to rollout the original patch and the wrong fix. And then you can reland the whole fixed patch when it is finished.
All patches related to this bug were rolled out - http://trac.webkit.org/changeset/107101 (original patch, buildfixes, gardening, fix, ...)
Created attachment 146254 [details] Patch
Comment on attachment 146254 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146254&action=review > Source/WebCore/platform/qt/FileSystemQt.cpp:222 > +#else > + struct statvfs volumeInfo; > + if (statvfs(path, &volumeInfo)) > + return 0; > + > + return static_cast<uint64_t>(volumeInfo.f_bavail) * static_cast<uint64_t>(volumeInfo.f_frsize); > +#endif IS this ensure to work on all non windows platforms (that we support)? :) > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:104 > + // The following variables are initialised to 0 because WebProcess::calculateCacheSizes might not > + // set them in some rare cases. Bug number for that?
(In reply to comment #61) > (From update of attachment 146254 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146254&action=review > > > Source/WebCore/platform/qt/FileSystemQt.cpp:222 > > +#else > > + struct statvfs volumeInfo; > > + if (statvfs(path, &volumeInfo)) > > + return 0; > > + > > + return static_cast<uint64_t>(volumeInfo.f_bavail) * static_cast<uint64_t>(volumeInfo.f_frsize); > > +#endif > > IS this ensure to work on all non windows platforms (that we support)? :) At least on all the POSIX systems, it should be supported. > > > Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:104 > > + // The following variables are initialised to 0 because WebProcess::calculateCacheSizes might not > > + // set them in some rare cases. > > Bug number for that? Not sure if this is a bug, actually. All the other ports simply initialize to 0 and some code paths in calculateCacheSizes do not reset the values. It might be possible that someone would like to override this and still have a non-zero default if calculateCacheSizes does not set any value.
Comment on attachment 146254 [details] Patch I have tested this on two different machines (one using sedkit, one using resworb environment) and gotten the same results with or without the patch apart from one test that got flaky with the patch (http/tests/xmlhttprequest/zero-length-response.html). Do you think we could commit this and watch the bots afterwards?
Comment on attachment 146254 [details] Patch Clearing flags on attachment: 146254 Committed r119965: <http://trac.webkit.org/changeset/119965>
Re-opened since this is blocked by 88771
Created attachment 146819 [details] Patch
Comment on attachment 146819 [details] Patch Build fix landed manually by Simon Hausmann in r119968.
All reviewed patches and the build fix have been landed, so closing the bug.