[webkit-reviews] review granted: [Bug 190640] Remove InjectedBundleBackForwardList : [Attachment 352506] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 190640: Remove InjectedBundleBackForwardList
https://bugs.webkit.org/show_bug.cgi?id=190640

Attachment 352506: Patch

https://bugs.webkit.org/attachment.cgi?id=352506&action=review




--- Comment #2 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 352506
  --> https://bugs.webkit.org/attachment.cgi?id=352506
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=352506&action=review

r=me with suggestion, assuming the bots are happy.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:1673
> +void WebPage::clearHistory()

This merely sends an async IPC to the UIProcess, asking it to clear its back
forward list. The UIProcess will then send async IPC to the WebProcess for each
history item removal.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:415
> +    WKBundleClearHistoryForTesting(m_page);

This merely asks the UIProcess to clear the history. Can we clear the history
directly from the UIProcess in TestController::resetStateToConsistentValues()
using WKBackForwardListClear()? It would end up through the same code path but
would do less IPC.

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:613
>  void TestRunner::clearBackForwardList()

Note that we may be able to drop this in a follow-up, now that you clear
history between tests. I think all the tests merely call this before starting,
although we should double check.
It'd be great if we could get rid of it because this would mean we could drop
WKBundleClearHistoryForTesting() too.