PlzNavigate layout tests: Output currently in WebFrameTestClient::willSendRequest() should be made from browser process |
||||
Issue descriptionIn Blink's layout tests, WebFrameTestClient::willSendRequest() makes output that several layout tests rely on seeing as part of their expectations. WebFrameTestClient::willSendRequest() is invoked from WebFrameTestProxy's implementation of the blink::WebFrameClient::willSendRequest() interface. Unfortunately in PlzNavigate blink::WebFrameClient::willSendRequest() can be invoked twice in the lifetime of a load. The first time is from RenderFrameImpl::BeginNavigation(); this occurs on renderer-initiated navigations only. The second time is from RenderFrameImpl::OnCommitNavigation(); this occurs on all successful navigations. When the output occurs twice in a given layout test that was expecting it only once, it causes that layout test to fail. Unfortunately, neither limiting the output to the first call nor to the second call works across all layout tests: - The first call is never made in the case of browser-initiated navigations, so some layout tests still fail if output is limited to the first call. - The second call is never made if the request is blocked, so some layout tests still fail if output is limited to the second call. The right thing to do for PlzNavigate seems to be to perform this output in the browser process rather than the renderer process.
,
Jul 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5a4fe32fc2b28a61e33f0cad0a7167a7ab88e80d commit 5a4fe32fc2b28a61e33f0cad0a7167a7ab88e80d Author: blundell <blundell@chromium.org> Date: Thu Jul 07 13:57:45 2016 PlzNavigate: Triage tests that are failing due to crbug.com/625765 These tests were found by a couple of hacky and incomplete renderer-side fixes to the problem. BUG= 625765 Review-Url: https://codereview.chromium.org/2121103002 Cr-Commit-Position: refs/heads/master@{#404152} [modify] https://crrev.com/5a4fe32fc2b28a61e33f0cad0a7167a7ab88e80d/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
,
Aug 12 2016
,
Jan 24 2017
Taking up for investigation
,
Jan 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fce540a08089df75d18cd4d719bec9ca03e88ac5 commit fce540a08089df75d18cd4d719bec9ca03e88ac5 Author: ananta <ananta@chromium.org> Date: Thu Jan 26 21:53:07 2017 PlzNavigate: Attempt to fix blink layout tests which fail due to duplicate output from WebFrameClient::willSendRequest. With PlzNavigate, the willSendRequest function is called in RenderFrameImpl::BeginNavigation() and again during OnCommitNavigation() when the request is loaded by blink. One option which was considered was to call willSendRequest() in OnCommitNavigation(). However the request may be blocked and hence that call may never happen. To workaround this I added a flag to track renderer initiated navigations to the RequestExtraData structure. We use this flag to determine whether the WebFrameTestClient::willSendRequest() function needs to early return or not. The assumption is that the logging would already be done when the navigation was initiated for renderer initiated navigations. No need to log when it is committed. The other changes are to add functionality to the WebTestDelegate interface for the runner to query this flag. There is one more test marked under the same bug http/tests/loading/redirect-methods.html. Does not appear to be the same issue. Will debug and log a new bug if required. BUG= 625765 Review-Url: https://codereview.chromium.org/2652123002 Cr-Commit-Position: refs/heads/master@{#446462} [modify] https://crrev.com/fce540a08089df75d18cd4d719bec9ca03e88ac5/components/test_runner/web_frame_test_client.cc [modify] https://crrev.com/fce540a08089df75d18cd4d719bec9ca03e88ac5/components/test_runner/web_test_delegate.h [modify] https://crrev.com/fce540a08089df75d18cd4d719bec9ca03e88ac5/content/child/request_extra_data.cc [modify] https://crrev.com/fce540a08089df75d18cd4d719bec9ca03e88ac5/content/child/request_extra_data.h [modify] https://crrev.com/fce540a08089df75d18cd4d719bec9ca03e88ac5/content/public/test/layouttest_support.h [modify] https://crrev.com/fce540a08089df75d18cd4d719bec9ca03e88ac5/content/renderer/render_frame_impl.cc [modify] https://crrev.com/fce540a08089df75d18cd4d719bec9ca03e88ac5/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/fce540a08089df75d18cd4d719bec9ca03e88ac5/content/shell/renderer/layout_test/blink_test_runner.h [modify] https://crrev.com/fce540a08089df75d18cd4d719bec9ca03e88ac5/content/test/layouttest_support.cc [modify] https://crrev.com/fce540a08089df75d18cd4d719bec9ca03e88ac5/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
,
Jan 26 2017
,
Mar 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/878b2e67ac952b00b2fcafc54867f61b25507923 commit 878b2e67ac952b00b2fcafc54867f61b25507923 Author: ananta <ananta@chromium.org> Date: Mon Mar 13 22:40:26 2017 PlzNavigate: Fix the http/tests/loading/307-after-303-after-post.html and the http/tests/loading/redirect-methods.html layout tests. These tests fail because willSendRequest() occurs before didStartProvisionalLoad(). In PlzNavigate willSendRequest is called when we send a navigation to the browser via BeginNavigation(). Proposed fix is to remember the navigation state in decidePolicyForNavigation() and invoke BeginNavigation() when we receive the didStartProvisionalLoad() notification. We pass the WebURLRequest in WebFrameClient::didStartProvisionalLoad() The other change is to remember the navigation state in a PendingNavigationInfo structure in RenderFrameImpl. This is populated in decidePolicyForNavigation() and released in didStartProvisionalLoad(). BUG= 647698 , 625765 Review-Url: https://codereview.chromium.org/2734633002 Cr-Commit-Position: refs/heads/master@{#456510} [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/content/renderer/render_frame_impl.cc [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/content/renderer/render_frame_impl.h [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/content/shell/test_runner/web_frame_test_client.cc [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/content/shell/test_runner/web_frame_test_client.h [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/content/shell/test_runner/web_frame_test_proxy.h [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/third_party/WebKit/Source/core/frame/LocalFrameClient.h [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/third_party/WebKit/Source/core/loader/EmptyClients.h [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/third_party/WebKit/Source/core/loader/FrameLoader.cpp [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/third_party/WebKit/Source/web/LocalFrameClientImpl.h [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/third_party/WebKit/Source/web/tests/WebDocumentSubresourceFilterTest.cpp [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/third_party/WebKit/Source/web/tests/WebFrameTest.cpp [modify] https://crrev.com/878b2e67ac952b00b2fcafc54867f61b25507923/third_party/WebKit/public/web/WebFrameClient.h |
||||
►
Sign in to add a comment |
||||
Comment 1 by carlosk@chromium.org
, Jul 5 2016