WPE fails to build with `-DENABLE_WPE_QT_API=ON` after changes done by r248846 in: * Source/WebKit/UIProcess/API/wpe/qt/WPEQtView.cpp * Source/WebKit/UIProcess/API/wpe/qt/WPEQtViewBackend.cpp ; to forces FastMalloc use. Related to: * https://bugs.webkit.org/show_bug.cgi?id=200611 * https://bugs.webkit.org/show_bug.cgi?id=200620
Created attachment 376867 [details] patch
Attachment 376867 [details] did not pass style-queue: ERROR: Source/WebKit/ChangeLog:30: Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: memory corruption [changelog/unwantedsecurityterms] [3] ERROR: Source/WebKit/UIProcess/API/wpe/qt/WPEQtView.cpp:442: Use 'WTF::makeUnique<>' instead of 'std::make_unique<>'. [runtime/wtf_make_unique] [4] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 376867 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=376867&action=review > Source/WebKit/UIProcess/API/wpe/qt/WPEQtView.cpp:442 > - std::unique_ptr<JavascriptCallbackData> data = makeUnique<JavascriptCallbackData>(callback, QPointer<WPEQtView>(this)); > + std::unique_ptr<JavascriptCallbackData> data = std::make_unique<JavascriptCallbackData>(callback, QPointer<WPEQtView>(this)); This doesn't look right. I think you can simply make JavascriptCallbackData WTF_MAKE_FAST_ALLOCATED and then makeUnique would work?
Created attachment 376869 [details] patch
(In reply to Philippe Normand from comment #3) > Comment on attachment 376867 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=376867&action=review > > > Source/WebKit/UIProcess/API/wpe/qt/WPEQtView.cpp:442 > > - std::unique_ptr<JavascriptCallbackData> data = makeUnique<JavascriptCallbackData>(callback, QPointer<WPEQtView>(this)); > > + std::unique_ptr<JavascriptCallbackData> data = std::make_unique<JavascriptCallbackData>(callback, QPointer<WPEQtView>(this)); > > This doesn't look right. I think you can simply make JavascriptCallbackData > WTF_MAKE_FAST_ALLOCATED and then makeUnique would work? Faster than my eyes, I uploaded the right version one minute ago. Sorry for the noise with the first one.
Attachment 376869 [details] did not pass style-queue: ERROR: Source/WebKit/ChangeLog:30: Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: memory corruption [changelog/unwantedsecurityterms] [3] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 376869 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=376869&action=review > Source/WebKit/ChangeLog:31 > + [WTF][JSC] Make JSC and WTF aggressively-fast-malloced > + https://bugs.webkit.org/show_bug.cgi?id=200611 > + ... > + Putting WebKit classes in FastMalloc has many benefits. > + > + 1. Simply, it is fast. > + 2. vmmap can tell the amount of memory used for WebKit. > + 3. bmalloc can isolate WebKit memory allocation from the rest of > + the world. This is useful since we can know more about what > + component is corrupting the memory from the memory corruption > + crash. Can you remove this? It would make the style bot happy and we can land this through cq.
Created attachment 376871 [details] patch
(In reply to Philippe Normand from comment #7) > Comment on attachment 376869 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=376869&action=review > > > Source/WebKit/ChangeLog:31 > > + [WTF][JSC] Make JSC and WTF aggressively-fast-malloced > > + https://bugs.webkit.org/show_bug.cgi?id=200611 > > + ... > > + Putting WebKit classes in FastMalloc has many benefits. > > + > > + 1. Simply, it is fast. > > + 2. vmmap can tell the amount of memory used for WebKit. > > + 3. bmalloc can isolate WebKit memory allocation from the rest of > > + the world. This is useful since we can know more about what > > + component is corrupting the memory from the memory corruption > > + crash. > > Can you remove this? It would make the style bot happy and we can land this > through cq. done and thanks for pointing the solution.
Comment on attachment 376871 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=376871&action=review > Source/WebKit/UIProcess/API/wpe/qt/WPEQtView.cpp:383 > + WTF_MAKE_FAST_ALLOCATED; Sorry, I've just found out there's a WTF_MAKE_STRUCT_FAST_ALLOCATED macro as well. Can you use it here?
Created attachment 376874 [details] patch
(In reply to Philippe Normand from comment #10) > Comment on attachment 376871 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=376871&action=review > > > Source/WebKit/UIProcess/API/wpe/qt/WPEQtView.cpp:383 > > + WTF_MAKE_FAST_ALLOCATED; > > Sorry, I've just found out there's a WTF_MAKE_STRUCT_FAST_ALLOCATED macro as > well. Can you use it here? Sure ... done
Comment on attachment 376874 [details] patch Thanks! :)
Comment on attachment 376874 [details] patch Clearing flags on attachment: 376874 Committed r248941: <https://trac.webkit.org/changeset/248941>
All reviewed patches have been landed. Closing bug.