Call URLLoaderClient::OnStartLoadingResponseBody earlier. |
||||
Issue descriptionAfter the URLLoaderClient interface is unbound in the browser process and bound again in the renderer process, it takes some amount of time to receive URLLoader::OnStartLoadingResponseBody(). It may worth sending the data pipe with the RenderFrameHostImpl::CommitNavigation() message. It would allow the Renderer process to commit the navigation faster. There are no strong reason to make URLLoader::OnReceiveResponse() and URLLoader::OnStartLoadingResponseBody() separate messages. This issue tracks progress in this direction.
,
Apr 10 2018
,
Apr 12 2018
FYI: I ran a benchmark with my temporary patch: https://ct.skia.org/results/cluster-telemetry/tasks/chromium_perf_runs/arthursonzogni-20180411144049/html/index.html I am not sure if it means anything. I saw another benchmark with an empty patch, ironically, it shows the same kind of difference: https://ct.skia.org/results/cluster-telemetry/tasks/chromium_perf_runs/farahcharab-20180410221318/html/index.html
,
Apr 12 2018
,
May 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c13c16a87cd7bfb4b4883311cd1a2b7343311a57 commit c13c16a87cd7bfb4b4883311cd1a2b7343311a57 Author: arthursonzogni <arthursonzogni@chromium.org> Date: Thu May 03 14:53:33 2018 Update two scroll tests. This updates the following tests: - NavigationControllerBrowserTest.ReloadWithUrlAnchor - NavigationControllerBrowserTest.ReloadWithUrlAnchorAndScroll This two tests are making a <div> to scroll because of a navigation to an anchor. Then the tests check the scroll position is restored after a reload. The issue is that it is not deterministic when the scrolled area is not the root area. It is working consistently only because the main resource's response is small (e.g <64ko) and FrameLoader::ProcessFragment() is called before FrameLoader::RestoreScrollPositionAndViewState(). This CL make the tests uses the root area instead of the <div>. --- Description of what happens in the current code: 1) When ProcessFragment() is called first: a) ProcessFragment(): Several scrollable layers are scrolled such that the anchor is displayed. It may includes the root one, but also some children. b) RestoreScrollPositionAndViewState(): The root layer is scrolled using what is stored in the HistoryItem. It overrides the scroll done in a) for the root layer. Children layers are not overridden. 2) When RestoreScrollPositionAndViewState() is called first: a) RestoreScrollPositionAndViewState(): The root layer is scrolled using what is stored in the HistoryItem. Children layers are not scrolled. b) ProcessFragment(): Nothing happens because InitialScrollState::did_restore_from_history has been set to true in a). The ProcessFragment() will skip the fragment scroll. At the end, the difference is that the children layers are scrolled in 1), but not in 2). --- Bug: 821877 , 831155, 839292 Change-Id: I11c74224ba42ad58814c4d4e185d7db189551775 Reviewed-on: https://chromium-review.googlesource.com/1039827 Reviewed-by: Jianpeng Chao <chaopeng@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Reviewed-by: Steve Kobes <skobes@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#555727} [modify] https://crrev.com/c13c16a87cd7bfb4b4883311cd1a2b7343311a57/content/browser/frame_host/navigation_controller_impl_browsertest.cc [modify] https://crrev.com/c13c16a87cd7bfb4b4883311cd1a2b7343311a57/content/test/data/navigation_controller/reload-with-url-anchor.html
,
May 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cfa684720709c0804d65805a1bd1790b695e1ebd commit cfa684720709c0804d65805a1bd1790b695e1ebd Author: arthursonzogni <arthursonzogni@chromium.org> Date: Mon May 07 17:18:00 2018 Update WPT test: url-in-tags.window.js The web-platform-test: FileAPI/url/url-in-tags.window.js relies on the load event to be dispatched after the page has scrolled to the anchor. This is not guaranteed. It is currently non-deterministic in Chrome. The page: https://scrolly-in-onload-event.glitch.me/ shows a case where the opposite happens. Doing task https://crbug.com/831155 caused this test to fail. Note: This test was working only on Chrome. Current status: https://wpt.fyi/FileAPI/url/url-in-tags.window.html There is a chance applying this patch will make it work on Safari as well. Related CL: https://chromium-review.googlesource.com/c/chromium/src/+/1042191 Bug: 831155 Change-Id: Ifa3f8e3d35cdb2635261248c3c9e9fd959f44340 Reviewed-on: https://chromium-review.googlesource.com/1046596 Reviewed-by: Steve Kobes <skobes@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#556479} [modify] https://crrev.com/cfa684720709c0804d65805a1bd1790b695e1ebd/third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/url-in-tags.window.js
,
May 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95336cb92b1337d4ad5d2a7fcc7409ec25b4a8ee commit 95336cb92b1337d4ad5d2a7fcc7409ec25b4a8ee Author: arthursonzogni <arthursonzogni@chromium.org> Date: Fri May 11 08:34:35 2018 Fix test: back-to-redirect-with-frame.php In this test, the following lines looks legitimate, but they are not: ~~~ window.onload = setTimeout(function() { window.location = "resources/go-back.html" }, 10); ~~~ The issue is that setTimeout is executed immediately instead of on load. This CL wraps it into a function. It also reduces the timeout from 10ms to 0ms. Bug: 831155 Change-Id: I7e35fcaeedf918a29fa12ae1f8292d07e08ac213 Reviewed-on: https://chromium-review.googlesource.com/1051908 Reviewed-by: Nate Chapin <japhet@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#557816} [modify] https://crrev.com/95336cb92b1337d4ad5d2a7fcc7409ec25b4a8ee/third_party/WebKit/LayoutTests/http/tests/navigation/back-to-redirect-with-frame.php
,
May 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4835a7de8159e7abde78afe271dc83ee08482971 commit 4835a7de8159e7abde78afe271dc83ee08482971 Author: arthursonzogni <arthursonzogni@chromium.org> Date: Mon May 14 08:55:12 2018 Fix tests: window.onload = setTimeout. This is similar to a previous CL: https://chromium-review.googlesource.com/c/chromium/src/+/1051908 All of these tests are using: window.onload = setTimeout([...], 0); The issue is that setTimeout is executed immediately instead of on load. Bug: 831155 Change-Id: I913b8c04de5d583ef87c91dfc23d4ba8be8ba642 Reviewed-on: https://chromium-review.googlesource.com/1055391 Reviewed-by: Nate Chapin <japhet@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#558229} [modify] https://crrev.com/4835a7de8159e7abde78afe271dc83ee08482971/third_party/WebKit/LayoutTests/http/tests/history/replaceState-then-fragment.html [modify] https://crrev.com/4835a7de8159e7abde78afe271dc83ee08482971/third_party/WebKit/LayoutTests/http/tests/history/resources/push-state-in-grandchild-grandchild.html [modify] https://crrev.com/4835a7de8159e7abde78afe271dc83ee08482971/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/async-xhr-revalidate-after-sync-xhr.html [modify] https://crrev.com/4835a7de8159e7abde78afe271dc83ee08482971/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/sync-xhr-revalidate-after-async-xhr-expected.txt [modify] https://crrev.com/4835a7de8159e7abde78afe271dc83ee08482971/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/sync-xhr-revalidate-after-async-xhr.html
,
May 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b3a06c1ba0f43f88e3764eb7b1d09b12f43a45c9 commit b3a06c1ba0f43f88e3764eb7b1d09b12f43a45c9 Author: arthursonzogni <arthursonzogni@chromium.org> Date: Mon May 14 11:52:35 2018 Fix flaky HeadlessWebContentsTest focus test. This test is a bit flaky: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=headless_browsertests&tests=FocusOfHeadlessWebContents_IsIndependent Furthermore, I am working on https://crbug.com/831155. It causes the frame to commit a navigation more quickly. As a result the frame may stop loading sooner. It causes this test to become even flakier. Why this test is flaky? It waits for the main frame to stop loading by calling WaitForLoad(). Then it waits for the main frame to gain focus by calling WaitForFocus(). In reality, it may gain focus before it stops loading. Since WaitForFocus() doesn't check whether or not the main frame is already focused before waiting, it may wait forever. The CL makes the tests wait for both event to happen, no matter the order. Related CL: https://chromium-review.googlesource.com/c/chromium/src/+/1026993 Bug: 831155 Change-Id: Id9002b3ee2a06ef17d670858688d28959f334e03 Reviewed-on: https://chromium-review.googlesource.com/1053777 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Eric Seckler <eseckler@chromium.org> Cr-Commit-Position: refs/heads/master@{#558262} [modify] https://crrev.com/b3a06c1ba0f43f88e3764eb7b1d09b12f43a45c9/headless/lib/headless_web_contents_browsertest.cc [modify] https://crrev.com/b3a06c1ba0f43f88e3764eb7b1d09b12f43a45c9/headless/test/headless_browser_test.cc [modify] https://crrev.com/b3a06c1ba0f43f88e3764eb7b1d09b12f43a45c9/headless/test/headless_browser_test.h
,
May 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/986ffe005c730dbdbd55bf3a1d9b7fc05b691c39 commit 986ffe005c730dbdbd55bf3a1d9b7fc05b691c39 Author: arthursonzogni <arthursonzogni@chromium.org> Date: Fri May 18 07:37:27 2018 FirstMeaningfulPaint: Ensure event is sent. The FirstMeaningfulPaintDetector method |CheckNetworkStable| is called whenever an active connection is removed. If the number of connection is low enough and is stable for long enough, the FirstMeaningfulPaint event will be sent. The current implementation does nothing if the document has not been parsed yet. In particular, for the main resource, this function is called when the load is completed. When it happens, the document may not have been parsed yet. It depends on how things are scheduled in blink. If there are no other sub resources to load, |CheckNetworkStable()| will not be called again and the FirstMeaningfulPaint will never be send. I am working on https://crbug.com/831155. It modifies how things are scheduled in Blink. Several tests related to this issue are failing: * SessionRestorePageLoadMetricsBrowserTest.LoadingAfterSessionRestore * SessionRestorePageLoadMetricsBrowserTest.MultipleSessionRestores * SessionRestorePageLoadMetricsBrowserTest.MultipleTabsSessionRestore * SessionRestorePageLoadMetricsBrowserTest.RestoreForeignTab This CL fixes this issue by calling |CheckNetworkStable()| when the document has been parsed. Bug: 831155 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I22c814280087dffd99e2eb6c02918dc1b8201fea Reviewed-on: https://chromium-review.googlesource.com/1064055 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#559829} [modify] https://crrev.com/986ffe005c730dbdbd55bf3a1d9b7fc05b691c39/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/page/page-lifecycleEvents-expected.txt [modify] https://crrev.com/986ffe005c730dbdbd55bf3a1d9b7fc05b691c39/third_party/blink/renderer/core/dom/document.cc [modify] https://crrev.com/986ffe005c730dbdbd55bf3a1d9b7fc05b691c39/third_party/blink/renderer/core/paint/first_meaningful_paint_detector.cc [modify] https://crrev.com/986ffe005c730dbdbd55bf3a1d9b7fc05b691c39/third_party/blink/renderer/core/paint/first_meaningful_paint_detector_test.cc
,
May 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c875e5d1fa83c83cf8cf3258454717c3b740e8b6 commit c875e5d1fa83c83cf8cf3258454717c3b740e8b6 Author: arthursonzogni <arthursonzogni@chromium.org> Date: Tue May 22 15:16:02 2018 navigate_to_204.html: Wait for window.load before navigating. I am working on https://crbug.com/831155. Some tests that are relying on how things are scheduled in Blink are failing. This CL makes the following test to keep working: * ExecuteScriptApiTest.FrameWithHttp204 What happened with this test? 1) Javascript test is started: test/data/extensions/api_test/executescript/http204/background.js 2) Function navigateToFrameAndWaitUntil204Loaded is executed. It navigates to: /extensions/api_test/executescript/http204/navigate_to_204.html?b.com 3) |location.href = "[...]page204.html"| happens before the document is fully loaded. 4) The navigation to page204.html cancels the previous one. FrameMsg_DroppedNavigation IPC is sent. 5) Test fails, it expected "page204.html" load to fail, but "navigate_to_204.html" failed first. Solution is to wait for the document to be fully loaded before navigating to page204.html. Bug: 831155 Change-Id: I8780a48877a8a6a67338aa4a1ae167952cb35fb8 Reviewed-on: https://chromium-review.googlesource.com/1065972 Reviewed-by: Camille Lamy <clamy@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#560592} [modify] https://crrev.com/c875e5d1fa83c83cf8cf3258454717c3b740e8b6/chrome/test/data/extensions/api_test/executescript/http204/navigate_to_204.html
,
May 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/48471218ba9ad3dffcd7657b670dfc17c5638022 commit 48471218ba9ad3dffcd7657b670dfc17c5638022 Author: arthursonzogni <arthursonzogni@chromium.org> Date: Thu May 24 22:34:53 2018 Fix race condition in frameElement.html. I am working on https://crbug.com/831155. Some tests that are relying on how things are scheduled in Blink are failing. There was a race condition in this test. The iframe "#fr2" was asked to navigate to a first URL, but soon after, a javascript function modifies the iframe.src to navigate elsewhere. What happened when it didn't work? 1) The first navigation starts. 2) The second navigation is scheduled (See blink's NavigationScheduler) 3) The first navigation commits. FrameLoader::CommitProvisionalLoad() executes: frame_->GetNavigationScheduler().Cancel(). It cancels the second navigation. This CL fixes the race condition. Bug: 831155 Change-Id: I2dd951140b4c5a671c749348ca0247f1901d8b77 Reviewed-on: https://chromium-review.googlesource.com/1069013 Commit-Queue: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#561659} [rename] https://crrev.com/48471218ba9ad3dffcd7657b670dfc17c5638022/third_party/WebKit/LayoutTests/external/wpt/html/browsers/windows/nested-browsing-contexts/frameElement.sub.html
,
May 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80348445e449a30d6f4d13b6592fee4044edab3b commit 80348445e449a30d6f4d13b6592fee4044edab3b Author: arthursonzogni <arthursonzogni@chromium.org> Date: Fri May 25 14:29:54 2018 Refresh frameElement.html test. - Removes unused things like <div id="log"></div>. It probably made sense at some point, but no more after the semi-automatic conversion to a WPT test. - Use more meaningful names. - Group elements (iframes/embed/objects) by tests. Bug: 831155 Change-Id: I7bb36132bb3e6be0fc952f4d2d7c1edb847878c1 Reviewed-on: https://chromium-review.googlesource.com/1069089 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#561865} [modify] https://crrev.com/80348445e449a30d6f4d13b6592fee4044edab3b/third_party/WebKit/LayoutTests/external/wpt/html/browsers/windows/nested-browsing-contexts/frameElement.sub.html
,
May 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2d1846bed03be552223791562288aea46c7c0c8a commit 2d1846bed03be552223791562288aea46c7c0c8a Author: arthursonzogni <arthursonzogni@chromium.org> Date: Mon May 28 12:28:02 2018 Ensure CSS :target is set before calling the load event. I am working on https://crbug.com/831155. It causes the frame to commit a navigation more quickly. As a result, some tests that are relying on how things are scheduled in blink are failling. This CL makes sure at least two WPT tests keep working: * external/wpt/dom/nodes/Element-matches.html * external/wpt/dom/nodes/Element-webkitMatchesSelector.html What happens in theses tests: In the frame.load event, document.querySelector(":target") is used. When an URL has an anchor, like in http://example.com#anchor, it returns the element with the "anchor" ID. The problem is that blink::Document::SetCSSTarget(Element*) may currently be called after the 'load' event is executed. The solution is simply to exchange two lines: 1) The one that will maybe send the 'load' event. frame_->GetDocument()->CheckCompleted(); 2) The one that will call Document::SetCSSTarget() ProcessFragment([...]) This CL is similar to: https://chromium-review.googlesource.com/c/chromium/src/+/1042191 It is the same solutioa to a different problem. This test is disabled because of http://crbug.com/843192 * svg/animations/viewspec-checkaspectparams.html Bug: 831155, 843523 , 843192 Change-Id: I8ecedd212d3ce30d252ec9c1b1f22214f7bc012a Reviewed-on: https://chromium-review.googlesource.com/1057507 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Steve Kobes <skobes@chromium.org> Cr-Commit-Position: refs/heads/master@{#562235} [modify] https://crrev.com/2d1846bed03be552223791562288aea46c7c0c8a/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/2d1846bed03be552223791562288aea46c7c0c8a/third_party/blink/renderer/core/loader/frame_loader.cc
,
May 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6023a17012ff4e86517c14ca0123cd1d345e7b53 commit 6023a17012ff4e86517c14ca0123cd1d345e7b53 Author: arthursonzogni <arthursonzogni@chromium.org> Date: Mon May 28 13:03:03 2018 Headless tests: Update navigation tracking to avoid a race condition. Why there is a the race condition: ================================== Given this document: ~~~ <html> <head> <script> document.location = 'http://www.example.com' </script> </head> <body> [...] </body> </html> ~~~ The scheduled navigation to http://www.example.com may starts before the document has finish loading. It depends on the size of the document and how things are scheduled in blink. FrameLoader::StartLoad() executes: ProgressTracker::ProgressStarted() which executes probe::frameStartedLoading() if and only if the frame is not loading currently. Since it can be called or not, it causes the HeadlessRenderTest to drop some navigations and a test to fail. I am working on https://crbug.com/831155. It makes some navigations to start sooner. This test is failing: HeadlessRenderBrowserTestClientRedirectChain.RunAsyncTest This CL keeps it working. Description of the patch: ========================= Using: 1) frameScheduledNavigation 2) frameStartedLoading 3) frameClearScheduledNavigation is not a correct way to track navigation. A new navigation can start before the previous one has finished loading. So the second frameStartedLoading may or may not be sent. This patch makes tests to continue to track whether or not some navigation have been scheduled using 1), but it stops trying to get informations from 2) and 3). More generally, I think using frameScheduledNavigation and frameClearScheduledNavigation should be avoided. At a first sight, I think it will miss some navigations, at least every browser initiated ones and maybe some renderer initiated too. Bug: 831155 Change-Id: I3bd18a6b20ee5a73e0116244066ee699822dec33 Reviewed-on: https://chromium-review.googlesource.com/1071630 Reviewed-by: Alex Clarke <alexclarke@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#562241} [modify] https://crrev.com/6023a17012ff4e86517c14ca0123cd1d345e7b53/headless/test/headless_render_browsertest.cc [modify] https://crrev.com/6023a17012ff4e86517c14ca0123cd1d345e7b53/headless/test/headless_render_test.cc [modify] https://crrev.com/6023a17012ff4e86517c14ca0123cd1d345e7b53/headless/test/headless_render_test.h
,
May 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a777927d9ca90acd0590bf12fc340f038657705f commit a777927d9ca90acd0590bf12fc340f038657705f Author: arthursonzogni <arthursonzogni@chromium.org> Date: Mon May 28 15:43:02 2018 Update inspector protocol test that relied on timing. I am working on https://crbug.com/831155. Some tests that are relying on how things are scheduled in Blink are failing. This CL makes 2 tests to keep working. * inspector-protocol/page/frameAttachedDetached.js * inspector-protocol/sessions/page-frame-events.js In these two tests, a frame is navigated twice. Immediately after the first load starts, a second navigation is requested. The tests expect Page.OnFrameStartedLoading to be called twice. The issue is the first load may not be finished when the second navigation is started. ProgressTracker::ProgressStarted() doesn't send the frameStartedLoading() if the frame is already loading something when FrameLoader::StartLoading is called. To fix the issue, the tests now wait for the frame to stop loading before navigating again. Bug: 831155 Change-Id: Iee71767be6d15dbf53c370bb414fe57015dd1df6 Reviewed-on: https://chromium-review.googlesource.com/1071791 Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Alex Clarke <alexclarke@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#562267} [modify] https://crrev.com/a777927d9ca90acd0590bf12fc340f038657705f/third_party/WebKit/LayoutTests/inspector-protocol/page/frameAttachedDetached.js [modify] https://crrev.com/a777927d9ca90acd0590bf12fc340f038657705f/third_party/WebKit/LayoutTests/inspector-protocol/sessions/page-frame-events.js
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/250cef84d714b3386a3910845842ea12c3a7abed commit 250cef84d714b3386a3910845842ea12c3a7abed Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Wed Jun 13 11:37:25 2018 Fix test: drag-files-to-editable-element.html The tested feature regressed, but no one realized it, because the test continued to pass. Instead of displaying the filenames in the <div>, a new navigation to the file happened. Starting the navigation causes the current Document to stop and the test to finish early. The tests finished before it has executed any assertions. This CL fixes the test, but not the regression. Bug: 821455, 831155 Change-Id: I61b9cca80677ca2caed72b0e9cf6074f73c2ad6a Reviewed-on: https://chromium-review.googlesource.com/1094879 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#566800} [modify] https://crrev.com/250cef84d714b3386a3910845842ea12c3a7abed/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/250cef84d714b3386a3910845842ea12c3a7abed/third_party/WebKit/LayoutTests/editing/pasteboard/drag-files-to-editable-element.html [add] https://crrev.com/250cef84d714b3386a3910845842ea12c3a7abed/third_party/WebKit/LayoutTests/editing/pasteboard/resources/drag-files-to-editable-element-fail
,
Jun 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3ae920b18b96d7fd53c32d9d70abfeba49c5754c commit 3ae920b18b96d7fd53c32d9d70abfeba49c5754c Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Fri Jun 15 14:30:21 2018 Update DomTreeExtractionBrowserTest. Assigned Node ID may not be the same for each executions of the test DomTreeExtractionBrowserTest.RunAsyncTest. This CL clears the value of backendNodeId to make this test more deterministic. backendNodeId uses DOMNodIds::IdForNode(). Here are two stack traces where the ID number 3 is assigned to a different Node. 02 blink::DOMNodeIds::IdForNode() 03 blink::InspectorDOMSnapshotAgent::VisitNode() 04 blink::InspectorDOMSnapshotAgent::VisitContainerChildren() 05 blink::InspectorDOMSnapshotAgent::VisitNode() 06 blink::InspectorDOMSnapshotAgent::getSnapshot() 07 blink::InspectorDOMSnapshotAgent::getSnapshot() 08 blink::protocol::DOMSnapshot::DispatcherImpl::getSnapshot() 09 blink::protocol::DOMSnapshot::DispatcherImpl::dispatch() 10 blink::protocol::UberDispatcher::dispatch() 11 blink::InspectorSession::DispatchProtocolMessage() 12 blink::WebDevToolsAgentImpl::Session::DispatchProtocolCommand() 13 blink::mojom::blink::DevToolsSessionStubDispatch::Accept() 02 blink::DOMNodeIds::IdForNode() 03 blink::CompositedLayerMapping::CreateGraphicsLayer() 04 blink::CompositedLayerMapping::CreatePrimaryGraphicsLayer() 05 blink::CompositedLayerMapping::CompositedLayerMapping() 06 blink::PaintLayer::EnsureCompositedLayerMapping() 07 blink::PaintLayerCompositor::AllocateOrClearCompositedLayerMapping() 08 blink::CompositingLayerAssigner::AssignLayersToBackingsInternal() 09 blink::CompositingLayerAssigner::Assign() 10 blink::PaintLayerCompositor::UpdateIfNeeded() 11 blink::PaintLayerCompositor::UpdateIfNeededRecursiveInternal() 12 blink::PaintLayerCompositor::UpdateIfNeededRecursive() 13 blink::LocalFrameView::UpdateLifecyclePhasesInternal() 14 blink::LocalFrameView::UpdateAllLifecyclePhases() 15 blink::PageAnimator::UpdateAllLifecyclePhases() 16 blink::PageWidgetDelegate::UpdateLifecycle() 17 blink::WebViewImpl::UpdateLifecycle() 18 blink::WebViewFrameWidget::UpdateLifecycle() 19 content::RenderWidget::UpdateVisualState() 20 content::RenderWidgetCompositor::UpdateLayerTreeHost() 21 cc::LayerTreeHost::RequestMainFrameUpdate() Currently, this test doesn't appear to be flaky. I am working on: https://crbug.com/831155. It causes the navigation to commit slightly faster. The test fails one time out of four, all the backendNodeId in getSnapshot() are shifted by one. Bug: 831155 Change-Id: I83a30202877bb65953957438284062f13fabeb6c Reviewed-on: https://chromium-review.googlesource.com/1101693 Reviewed-by: Alex Clarke <alexclarke@chromium.org> Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#567640} [modify] https://crrev.com/3ae920b18b96d7fd53c32d9d70abfeba49c5754c/headless/lib/dom_tree_extraction_expected_nodes.txt [modify] https://crrev.com/3ae920b18b96d7fd53c32d9d70abfeba49c5754c/headless/lib/headless_devtools_client_browsertest.cc
,
Jun 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/743262940430fc58261bbcf28658d500a74721b6 commit 743262940430fc58261bbcf28658d500a74721b6 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Mon Jun 18 08:45:55 2018 Update test event-source-constructor.html This CL update the blink layout test: fast/eventsource/eventsource-constructor.html Depending on timing, the third EventSource: ~~~ new EventSource("http://disallowed.example.com/"); ~~~ may start before the test finishes. It results in a new console error message displayed. This CL prevent the console error message to be displayed. Note: I am working on https://crbug.com/831155. It causes the frame to commit a navigation slightly faster. As a result, some tests that are relying on how things are scheduled in blink are failing. Bug: 831155 Change-Id: I8a6bf0c1f1203be1e529b7eb6e3192849f7c518d Reviewed-on: https://chromium-review.googlesource.com/1102324 Reviewed-by: Mike West <mkwst@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#567952} [modify] https://crrev.com/743262940430fc58261bbcf28658d500a74721b6/third_party/WebKit/LayoutTests/fast/eventsource/eventsource-constructor.html
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3241281561a9eaa49d4471babfb6fa4ff562513a commit 3241281561a9eaa49d4471babfb6fa4ff562513a Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Tue Jun 19 09:13:24 2018 Fix potentially flaky inspector-protocol tests. Update the blink layout test: http/tests/inspector-protocol/frameScheduledNavigation.js This CL is similar to: * https://chromium-review.googlesource.com/c/chromium/src/+/1071630 * https://chromium-review.googlesource.com/c/chromium/src/+/1071791 Depending on timing and the size of the main resource, new navigations initiated in <script> may starts before the document is fully loaded. In this case, frameStartedLoading is not fired because the load didn't stop in between. Bug: 831155 Change-Id: I872b7ff9b332893c0b57f921f992e348276e9d42 Reviewed-on: https://chromium-review.googlesource.com/1102681 Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#568376} [modify] https://crrev.com/3241281561a9eaa49d4471babfb6fa4ff562513a/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/page/frameScheduledNavigation-expected.txt [modify] https://crrev.com/3241281561a9eaa49d4471babfb6fa4ff562513a/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/page/frameScheduledNavigation.js
,
Jun 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c0327d09b6e4444caca64f3bd16ec0bd20034049 commit c0327d09b6e4444caca64f3bd16ec0bd20034049 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Mon Jun 25 10:40:27 2018 Call Document::CheckCompleted() after CancelParsing.. Document::CancelParsing() may cause Document::ShouldComplete() to return true. Document::CheckCompleted() needs to be called so that its DocumentLoader is marked to have finished its load. A lot of tests don't expect DidStopLoading() and DidStartLoading() to be fired when a new DocumentLoader starts and replaces the previous one. In FrameLoader::StartLoad(), Document::CancelParsing() is moved after the creation of the new provisional DocumentLoader so that the FrameLoader isn't temporarily without any non loading Document. If that happened, DidStopLoading() would be fired. There was also one small bug with site isolation. The condition |have_to_check_if_parent_is_completed| in HTMLFrameOwnerElement::DisconnectContentFrame() was too strict. The parent document may have completed its load, but not have sent DidStopLoading yet because its childs were still loading. Note: I am working on https://crbug.com/831155. Some tests that are relying on how things are scheduled in Blink are failing. This CL makes DownloadTest.TestMultipleDownloadsRequests to keep working. Here, while the current document is loading, a new navigation happens and cancels the parser of the current document without calling Document::CheckCompleted() on it. Then, it turns out the navigation is a download. The navigation is dropped. In the end the main frame doesn't know its iframe has finished loading and the test fails. Bug: 831155 Change-Id: Ib2de108e25991349b44ec4071fe61d57ca6e85a1 Reviewed-on: https://chromium-review.googlesource.com/1107808 Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#570002} [modify] https://crrev.com/c0327d09b6e4444caca64f3bd16ec0bd20034049/content/browser/download/download_browsertest.cc [modify] https://crrev.com/c0327d09b6e4444caca64f3bd16ec0bd20034049/third_party/blink/renderer/core/html/html_frame_owner_element.cc [modify] https://crrev.com/c0327d09b6e4444caca64f3bd16ec0bd20034049/third_party/blink/renderer/core/loader/frame_loader.cc
,
Jul 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7521deccbb7d3d4a7700e6e007e679a8706d8be0 commit 7521deccbb7d3d4a7700e6e007e679a8706d8be0 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Mon Jul 09 12:31:04 2018 Transmit the response's body datapipe in CommitNavigation(). # Summary of the current behavior: When a navigation occurs, the mojo::URLLoaderClient interface is first bound to the content::NavigationURLLoaderImpl in the browser process. When the browser knows in which renderer process the navigation will commit, the interface is bound to the content::URLLoaderClientImpl in the renderer process to continue the navigation. The switching from one to another happens when mojo::URLLoaderClient::OnReceiveResponse() is called. This is described here: https://goo.gl/Rrrc7n # What are we trying to do: After the mojo::URLLoaderClient interface is unbound in the browser process and bound again in the renderer process, it takes some amount of time to receive URLLoader::OnStartLoadingResponseBody(). It may be worth sending the data pipe with RenderFrameHostImpl::CommitNavigation() and give it to the renderer process immediatly instead of waiting for it. It would allow the Renderer process to get the main resource's data earlier, especially when the renderer process is busy. Note: It doesn't look like there are any strong reasons to make URLLoaderClient::OnReceiveResponse(response_head) and URLLoaderClient::OnStartLoadingResponseBody(response_body) separate messages, maybe at some point, they could be merged into: URLLoaderClient::OnReceiveResponse(response_head, response_body) # What this CL does: This CL makes the switch to happen in URLLoaderClient::OnStartLoadingResponseBody(). The data pipe is transmitted in CommitNavigation(). This is enabled/disabled using the NavigationImmediateResponse experiment The goal is to look for performance improvement or regressions on Canary. Bug: 831155 Change-Id: Id6cf667fdc1482baf27f41aab754e58d9a5a569b Reviewed-on: https://chromium-review.googlesource.com/1100830 Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#573276} [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/chrome/browser/ui/views/location_bar/location_bar_view_browsertest.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/browser_side_navigation_browsertest.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/frame_host/navigation_request.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/frame_host/navigation_request.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/frame_host/render_frame_host_impl.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/loader/mojo_async_resource_handler.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/loader/mojo_async_resource_handler.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/loader/mojo_async_resource_handler_unittest.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/loader/navigation_url_loader_delegate.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/loader/navigation_url_loader_impl.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/loader/navigation_url_loader_impl.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/browser/loader/resource_dispatcher_host_impl.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/common/frame.mojom [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/common/navigation_client.mojom [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/public/common/browser_side_navigation_policy.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/public/common/browser_side_navigation_policy.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/public/common/content_features.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/public/common/content_features.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/renderer/loader/navigation_response_override_parameters.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/renderer/loader/resource_dispatcher.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/renderer/loader/web_url_loader_impl.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/renderer/navigation_client.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/renderer/navigation_client.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/renderer/render_frame_impl.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/renderer/render_frame_impl.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/test/mock_navigation_client_impl.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/test/mock_navigation_client_impl.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/test/test_navigation_url_loader.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/test/test_navigation_url_loader_delegate.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/test/test_navigation_url_loader_delegate.h [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/content/test/test_render_frame.cc [modify] https://crrev.com/7521deccbb7d3d4a7700e6e007e679a8706d8be0/third_party/blink/renderer/platform/loader/fetch/raw_resource.cc
,
Jul 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1 commit 7178f5c1387636c7e5dfec90a533bcadb5c0c0c1 Author: Philip Rogers <pdr@chromium.org> Date: Mon Jul 09 18:52:59 2018 Revert "Transmit the response's body datapipe in CommitNavigation()." This reverts commit 7521deccbb7d3d4a7700e6e007e679a8706d8be0. Reason for revert: Caused failures on fast/history/history-back-initial-vs-final-url.html (and possibly other tests, see https://crbug.com/861808 ) Original change's description: > Transmit the response's body datapipe in CommitNavigation(). > > # Summary of the current behavior: > > When a navigation occurs, the mojo::URLLoaderClient interface is first > bound to the content::NavigationURLLoaderImpl in the browser process. > When the browser knows in which renderer process the navigation will > commit, the interface is bound to the content::URLLoaderClientImpl in > the renderer process to continue the navigation. The switching from one > to another happens when mojo::URLLoaderClient::OnReceiveResponse() is > called. This is described here: https://goo.gl/Rrrc7n > > # What are we trying to do: > > After the mojo::URLLoaderClient interface is unbound in the browser > process and bound again in the renderer process, it takes some amount of > time to receive URLLoader::OnStartLoadingResponseBody(). It may be worth > sending the data pipe with RenderFrameHostImpl::CommitNavigation() and > give it to the renderer process immediatly instead of waiting for it. It > would allow the Renderer process to get the main resource's data > earlier, especially when the renderer process is busy. > > Note: It doesn't look like there are any strong reasons to make > URLLoaderClient::OnReceiveResponse(response_head) and > URLLoaderClient::OnStartLoadingResponseBody(response_body) > separate messages, maybe at some point, they could be merged into: > URLLoaderClient::OnReceiveResponse(response_head, response_body) > > # What this CL does: > > This CL makes the switch to happen in > URLLoaderClient::OnStartLoadingResponseBody(). The data pipe is > transmitted in CommitNavigation(). > > This is enabled/disabled using the NavigationImmediateResponse experiment > The goal is to look for performance improvement or regressions on > Canary. > > Bug: 831155 > Change-Id: Id6cf667fdc1482baf27f41aab754e58d9a5a569b > Reviewed-on: https://chromium-review.googlesource.com/1100830 > Reviewed-by: Yutaka Hirano <yhirano@chromium.org> > Reviewed-by: Peter Kasting <pkasting@chromium.org> > Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> > Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> > Cr-Commit-Position: refs/heads/master@{#573276} TBR=pkasting@chromium.org,kinuko@chromium.org,nasko@chromium.org,yhirano@chromium.org,clamy@chromium.org,arthursonzogni@chromium.org Change-Id: I22eac2d660f9cf1d8128331b547c304af729a85a No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 831155 Reviewed-on: https://chromium-review.googlesource.com/1129659 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#573392} [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/chrome/browser/ui/views/location_bar/location_bar_view_browsertest.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/browser_side_navigation_browsertest.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/frame_host/navigation_request.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/frame_host/navigation_request.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/frame_host/render_frame_host_impl.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/loader/mojo_async_resource_handler.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/loader/mojo_async_resource_handler.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/loader/mojo_async_resource_handler_unittest.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/loader/navigation_url_loader_delegate.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/loader/navigation_url_loader_impl.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/loader/navigation_url_loader_impl.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/browser/loader/resource_dispatcher_host_impl.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/common/frame.mojom [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/common/navigation_client.mojom [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/public/common/browser_side_navigation_policy.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/public/common/browser_side_navigation_policy.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/public/common/content_features.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/public/common/content_features.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/renderer/loader/navigation_response_override_parameters.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/renderer/loader/resource_dispatcher.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/renderer/loader/web_url_loader_impl.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/renderer/navigation_client.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/renderer/navigation_client.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/renderer/render_frame_impl.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/renderer/render_frame_impl.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/test/mock_navigation_client_impl.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/test/mock_navigation_client_impl.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/test/test_navigation_url_loader.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/test/test_navigation_url_loader_delegate.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/test/test_navigation_url_loader_delegate.h [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/content/test/test_render_frame.cc [modify] https://crrev.com/7178f5c1387636c7e5dfec90a533bcadb5c0c0c1/third_party/blink/renderer/platform/loader/fetch/raw_resource.cc
,
Jul 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ae5ca38f28d3e136c7def1fe7444afed1db7ea9a commit ae5ca38f28d3e136c7def1fe7444afed1db7ea9a Author: Dmitry Gozman <dgozman@chromium.org> Date: Thu Jul 12 18:50:48 2018 FrameLoader: move CancelParsing and CheckCompleted earlier This way we do it before creating new provisional loader, which will help in the future when there would be no provisional loader. Added Document::Abort method which encapsulates CancelParsing and CheckCompleted and also does not issue undesired DidFinishNavigation. Bug: 831155 Change-Id: Ica5d968508d7aed7bb07125702ceeaeb6f389497 Reviewed-on: https://chromium-review.googlesource.com/1119262 Commit-Queue: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#574650} [modify] https://crrev.com/ae5ca38f28d3e136c7def1fe7444afed1db7ea9a/third_party/WebKit/LayoutTests/http/tests/loading/onreadystatechange-detach-expected.txt [add] https://crrev.com/ae5ca38f28d3e136c7def1fe7444afed1db7ea9a/third_party/WebKit/LayoutTests/http/tests/loading/onreadystatechange-window-stop.html [modify] https://crrev.com/ae5ca38f28d3e136c7def1fe7444afed1db7ea9a/third_party/blink/renderer/core/dom/document.cc [modify] https://crrev.com/ae5ca38f28d3e136c7def1fe7444afed1db7ea9a/third_party/blink/renderer/core/dom/document.h [modify] https://crrev.com/ae5ca38f28d3e136c7def1fe7444afed1db7ea9a/third_party/blink/renderer/core/loader/frame_loader.cc
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/83b84428f235e8fb47c72acb621d1d382ee866f0 commit 83b84428f235e8fb47c72acb621d1d382ee866f0 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Mon Jul 30 08:08:36 2018 Fix flaky xssAuditor test. The layout test: object-src-inject.html is flaky. Sometimes, this line is printed: ``` CONSOLE MESSAGE: Blink Test Plugin: initializing ``` It is fired from BlinkTestInstance::Init(...); It is racing with the end of the layout test. This CL disables console error dump to avoid this issue. Bug: 862959 , 831155 Change-Id: I912e71b9ff8b7b5ccd7a4716f111f2017493e37a Reviewed-on: https://chromium-review.googlesource.com/1135003 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#578994} [modify] https://crrev.com/83b84428f235e8fb47c72acb621d1d382ee866f0/third_party/WebKit/LayoutTests/http/tests/security/xssAuditor/object-src-inject.html
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/16edd27467d3490c9f7f46b2b6661d492e780c57 commit 16edd27467d3490c9f7f46b2b6661d492e780c57 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Mon Jul 30 08:58:42 2018 Fix test: http/tests/navigation/back-send-referrer.html The navigation may be triggered before the load event. When it happens, the history entry must replace the current one instead of being appended to it. Then history.go(-1) fails in the next document. This CL make the navigation to happen after the load event. Bug: 862580 , 831155 Change-Id: I4a48d50f5899e3fd65c74d9a89ab836ae94ba7be Reviewed-on: https://chromium-review.googlesource.com/1133261 Reviewed-by: Nate Chapin <japhet@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#578998} [modify] https://crrev.com/16edd27467d3490c9f7f46b2b6661d492e780c57/third_party/WebKit/LayoutTests/http/tests/navigation/back-send-referrer.html
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5f32e29a69aaeb7c3cb9a29835669cc8116ed404 commit 5f32e29a69aaeb7c3cb9a29835669cc8116ed404 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Mon Jul 30 09:42:52 2018 Fix 2 flaky blink layout tests. The test http/tests/misc/adopt-iframe-src-attr-after-remove.html as always been a bit flaky: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=http%2Ftests%2Fmisc%2Fadopt-iframe-src-attr-after-remove.html&testType=webkit_layout_tests The reason is that navigations triggered by a script before the onload handler don't generate back/forward entries. See third_party/blink/public/web/web_history_commit_type.h Sometimes, this test was fast enough so that no history entry was created. Then history.back() failed and it ended with a timeout. This is the same for: http/tests/misc/resource-timing-iframe-restored-from-history.html Bug: 862580 , 831155 Change-Id: I3b8243c5f43c5043de8e23acfbe561a648c0f115 Reviewed-on: https://chromium-review.googlesource.com/1133170 Reviewed-by: Nate Chapin <japhet@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#579001} [modify] https://crrev.com/5f32e29a69aaeb7c3cb9a29835669cc8116ed404/third_party/WebKit/LayoutTests/fast/history/history-back-initial-vs-final-url.html [modify] https://crrev.com/5f32e29a69aaeb7c3cb9a29835669cc8116ed404/third_party/WebKit/LayoutTests/http/tests/misc/resource-timing-iframe-restored-from-history.html
,
Aug 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/36f5bb5e7c843d8768b86977fc879135da365eea commit 36f5bb5e7c843d8768b86977fc879135da365eea Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Fri Aug 31 08:09:27 2018 Update LayoutTest /history/redirect-js-* (11 tests) The current behavior of chrome is the following one: --- When a javascript navigation is triggered, if it happens: * before the load event, replace the current history entry. * after the load event, append a new history entry. --- The first case is sometimes called "javascript-redirect". The current set of tests uses two kind of tests. One with a timeout of 2 seconds. This one is expected to happens after the load event. Another one with a timeout of 0 seconds. We don't really know this one is expected to happens. This is flaky. It usually happens after the load event, but it is non deterministic. This CLs removes: * /http/tests/history/redirect-js-*-0-seconds}.html * /http/tests/history/redirect-js-*-2-seconds}.html And completes: * /http/tests/history/redirect-js-*-before-load}.html * /http/tests/history/redirect-js-*-after-load}.html It modifies the tests so that they really depend on the load event, not on timing, which is not reliable. Bug: 862591 , 831155 Change-Id: I3d568019c8616e6b52b9db554ad95ed312bd82d9 Reviewed-on: https://chromium-review.googlesource.com/1185094 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Cr-Commit-Position: refs/heads/master@{#587965} [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-document-location-2-seconds.html [rename] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-document-location-after-load-expected.txt [add] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-document-location-after-load.html [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-form-submit-0-seconds-expected.txt [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-form-submit-0-seconds.html [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-form-submit-2-seconds-expected.txt [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-form-submit-2-seconds.html [copy] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-form-submit-after-load-expected.txt [add] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-form-submit-after-load.html [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-2-seconds-expected.txt [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-2-seconds.html [rename] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-after-load-expected.txt [add] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-after-load.html [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-assign-0-seconds.html [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-assign-2-seconds-expected.txt [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-assign-2-seconds.html [rename] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-assign-after-load-expected.txt [add] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-assign-after-load.html [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-href-2-seconds.html [rename] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-href-after-load-expected.txt [add] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-href-after-load.html [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-replace-0-seconds.html [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-replace-2-seconds.html [rename] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-replace-after-load-expected.txt [add] https://crrev.com/36f5bb5e7c843d8768b86977fc879135da365eea/third_party/WebKit/LayoutTests/http/tests/history/redirect-js-location-replace-after-load.html [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/reload-during-load-after-load-event-expected.txt [delete] https://crrev.com/3b06b81f4b9d5eafc7f65821522bc19bc362a93f/third_party/WebKit/LayoutTests/http/tests/history/reload-during-load-after-load-event.html
,
Sep 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2c76a0fac8022f693c0ee86a18676734b79e8512 commit 2c76a0fac8022f693c0ee86a18676734b79e8512 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Tue Sep 04 16:49:09 2018 Fix flaky LayoutTest: adopt-iframe-src-attr-after-remove.html This test was: 1) using non valid HTML syntax (line 3). 2) using (for no reason) named access on the window object https://html.spec.whatwg.org/#named-access-on-the-window-object The iframe with id "iframe" is globally accessible using "window.iframe" (or implicitly "iframe"). This is more or less deprecated in favor of using document.getElementById("iframe"). It causes the test to be flaky (see below for understanding why). This CL removes 1) and 2). Why this test is flaky? --------------------------- Line 37 in DOMContentLoaded handler, adoptAfterLoaded is executed after a 1ms timeout. If the 'load' event is fired before 1ms, the iframe is removed and window.iframe is now undefined. Then in adoptAfterLoaded, 'ifr' is undefined and is dereferenced line 24, the script fails just before calling testRunner.notifyDone(). Bug: 831155, 861808 Change-Id: I131dfb3b74c6d3fea1c412b219008a1be544a4f7 Reviewed-on: https://chromium-review.googlesource.com/1202282 Reviewed-by: Camille Lamy <clamy@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#588546} [modify] https://crrev.com/2c76a0fac8022f693c0ee86a18676734b79e8512/third_party/WebKit/LayoutTests/http/tests/misc/adopt-iframe-src-attr-after-remove.html
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a63ed4ed0dbbd23523479b1732815cf968ad3e5 commit 4a63ed4ed0dbbd23523479b1732815cf968ad3e5 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Thu Oct 04 16:38:39 2018 Fix flaky test open-with-pending-load2 There is a race in between: 1) The end of the test. 2) The XHR 'abort' event handlers executed. There is a PostTask in between: A) blink::XMLHttpRequest::HandleDidCancel() B) blink::XMLHttpRequest::HandleRequestError() FrameLoader::FinishedParsing may be interleaved in between A) and B). Make use of testRunner.{waitUntilDone, notifyDone} to get reliable results. Bug: 831155 Change-Id: Ibe1ad9020319aae7268f331b2dba0911035789e2 Reviewed-on: https://chromium-review.googlesource.com/c/1261520 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#596704} [modify] https://crrev.com/4a63ed4ed0dbbd23523479b1732815cf968ad3e5/third_party/WebKit/LayoutTests/fast/dom/Document/open-with-pending-load2-expected.txt [modify] https://crrev.com/4a63ed4ed0dbbd23523479b1732815cf968ad3e5/third_party/WebKit/LayoutTests/fast/dom/Document/open-with-pending-load2.html
,
Nov 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fecf153880556829aa643a33d7e015184ad8b1af commit fecf153880556829aa643a33d7e015184ad8b1af Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Thu Nov 08 08:44:36 2018 In tests, always send response's body data pipe, even if empty. After sending: URLLoaderClient::OnReceiveResponse(response_head), not all URLLoader are sending: URLLoaderClient::OnStartLoadingResponseBody(response_body). It even happens to send URLLoaderClient::OnComplete(net::OK) after that, which may confuse the URLLoaderClient. Most URLLoader not sending a response's body datapipe are the one with an empty response. For consistency, always send a response's body datapipe, even if it doesn't contains data. This CL updates every tests not aligned with this. Once every URLLoader and tests are consistent. DCHECK enforcing this will be added. This CL is a prerequisite for: https://chromium-review.googlesource.com/c/chromium/src/+/1172290 Bug: 826868 , 831155 Change-Id: Id3434ea442c93b47bd200d15322cc21cd0c0b89f Reviewed-on: https://chromium-review.googlesource.com/c/1323092 Reviewed-by: Evan Stade <estade@chromium.org> Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#606386} [modify] https://crrev.com/fecf153880556829aa643a33d7e015184ad8b1af/chrome/browser/devtools/devtools_sanity_browsertest.cc [modify] https://crrev.com/fecf153880556829aa643a33d7e015184ad8b1af/chrome/browser/ui/views/location_bar/location_bar_view_browsertest.cc [modify] https://crrev.com/fecf153880556829aa643a33d7e015184ad8b1af/content/renderer/loader/resource_dispatcher_unittest.cc
,
Nov 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2dfa163da62c53bcfbe1b87240ac1368cae66c67 commit 2dfa163da62c53bcfbe1b87240ac1368cae66c67 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Thu Nov 08 16:04:06 2018 AboutURLLoader: send an empty response's body. This CL is a prerequisite for: https://chromium-review.googlesource.com/c/chromium/src/+/1172290 After sending: URLLoaderClient::OnReceiveResponse(response_head), not all URLLoader are sending: URLLoaderClient::OnStartLoadingResponseBody(response_body). It even happens to send URLLoaderClient::OnComplete(net::OK) after that, which may confuse the URLLoaderClient. This CL updates the AboutURLLoader class. The goal is to align every URLLoaders, they will always send a data pipe after OnReceiveResponse(). Bug: 826868 , 831155 Change-Id: Iab3be1ed84011793a23eac304621d0ec0fc7f47e Reviewed-on: https://chromium-review.googlesource.com/c/1323070 Reviewed-by: Camille Lamy <clamy@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#606485} [modify] https://crrev.com/2dfa163da62c53bcfbe1b87240ac1368cae66c67/content/browser/loader/navigation_url_loader_impl.cc
,
Nov 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c25c2041ed579bcea8215c1c40063275ac86144f commit c25c2041ed579bcea8215c1c40063275ac86144f Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Mon Nov 12 16:54:32 2018 Simplify BlobURLLoader and always send OnStartLoadingResponseBody. This is a sort of prerequisite for: https://chromium-review.googlesource.com/c/chromium/src/+/1172290 The main goal is after sending: URLLoaderClient::OnReceiveResponse(response_header) to ALWAYS send: URLLoaderClient::OnStartLoadingResponseBody(response_body) Some URLLoader, when the response's body body is empty, don't even try to send the response_body data pipe. In the URLLoaderClient, not having to handle the case with no data pipe would be a nice simplification. What this CL does: 1) In BlobUrlLoader, create the data pipe at ::Start() time. 2) Pass the producer handle to the MojoBlobCreate at construction time. 3) Pass the consumer handle to OnStartLoadingResponseBody(), immediately after OnReceiveResponse(). Some code simplification was possible. There is no more need to implement PassDataPipe() in BlobURLLoader, ReaderDelegate and DataPipeReaderDelegate. There is no more need to give them the pipe at construction time neither. Bug: 826868 , 831155 Change-Id: I827cb39bdd782624ff362874e98be90cab0d47f3 Reviewed-on: https://chromium-review.googlesource.com/c/1329741 Reviewed-by: Marijn Kruisselbrink <mek@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#607266} [modify] https://crrev.com/c25c2041ed579bcea8215c1c40063275ac86144f/storage/browser/blob/blob_impl.cc [modify] https://crrev.com/c25c2041ed579bcea8215c1c40063275ac86144f/storage/browser/blob/blob_url_loader.cc [modify] https://crrev.com/c25c2041ed579bcea8215c1c40063275ac86144f/storage/browser/blob/blob_url_loader.h [modify] https://crrev.com/c25c2041ed579bcea8215c1c40063275ac86144f/storage/browser/blob/mojo_blob_reader.cc [modify] https://crrev.com/c25c2041ed579bcea8215c1c40063275ac86144f/storage/browser/blob/mojo_blob_reader.h
,
Nov 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b6d920e2f3a2457057853f2e8791916ffffe55bf commit b6d920e2f3a2457057853f2e8791916ffffe55bf Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Mon Nov 12 17:05:23 2018 Service worker: match OnStartLoadingResponseBody with OnReceiveResponse. Some URLLoaders aren't always sending a response's body datapipe to their client after sending URLLoaderClient::OnReceiveResponse(response_head). It even happens to send URLLoaderClient::OnComplete(net::OK) after that. Most of the time, they do not send a datapipe, because the response's body is empty. The goal is to align URLLoaders so that they will always send a data pipe by using OnStartLoadingResponseBody(response_body). This CL is about ServiceWorker's URLLoaders. This CL is a prerequisite for: https://chromium-review.googlesource.com/c/chromium/src/+/1172290 Bug: 826868 , 831155 Change-Id: Ib28fc2067240d611ec149099b5365f70610a9369 Reviewed-on: https://chromium-review.googlesource.com/c/1323109 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#607273} [modify] https://crrev.com/b6d920e2f3a2457057853f2e8791916ffffe55bf/content/browser/service_worker/service_worker_navigation_loader.cc [modify] https://crrev.com/b6d920e2f3a2457057853f2e8791916ffffe55bf/content/browser/service_worker/service_worker_navigation_loader.h [modify] https://crrev.com/b6d920e2f3a2457057853f2e8791916ffffe55bf/content/renderer/service_worker/service_worker_subresource_loader.cc [modify] https://crrev.com/b6d920e2f3a2457057853f2e8791916ffffe55bf/content/renderer/service_worker/service_worker_subresource_loader.h
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9aca9fc8da7772338917bc310af2410997963044 commit 9aca9fc8da7772338917bc310af2410997963044 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Thu Nov 29 10:47:02 2018 Update MojoAsyncResourceHandler. Always send response's body datapipe. Instead of creating the response's body data pipe during the first call to OnWillRead(), it is now done in OnWillStart(). The data pipe consumer handle is available in OnResponseStarted(). It allows the MojoAsyncResourceHandler to call: * URLLoader::OnReceiveResponse(response_headers) * URLLoader::OnStartLoadingResponseBody(response_body) at the same time. The goal is to guarantee the response's body is always sent, even if it doesn't contains any data. Bug: 831155, 826868 Change-Id: I85d84d05ad3fd362d96394834e14297d550f5f13 Reviewed-on: https://chromium-review.googlesource.com/c/1352254 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#612137} [modify] https://crrev.com/9aca9fc8da7772338917bc310af2410997963044/content/browser/loader/mojo_async_resource_handler.cc [modify] https://crrev.com/9aca9fc8da7772338917bc310af2410997963044/content/browser/loader/mojo_async_resource_handler.h [modify] https://crrev.com/9aca9fc8da7772338917bc310af2410997963044/content/browser/loader/mojo_async_resource_handler_unittest.cc
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b78c35086d661489c42042e94362294cf3e9a15b commit b78c35086d661489c42042e94362294cf3e9a15b Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Thu Nov 29 11:50:48 2018 MimeSniffingURLLoader: Always forward empty response's body. This is similar to: * https://chromium-review.googlesource.com/c/chromium/src/+/1323109 * https://chromium-review.googlesource.com/c/chromium/src/+/1329163 * https://chromium-review.googlesource.com/c/chromium/src/+/1323070 * https://chromium-review.googlesource.com/c/chromium/src/+/1323092 On request success, it makes the MimeSniffingURLLoader to always send a response's body datapipe, even when it is empty. It also adds a DCHECK to ensure the |source_url_loader| is also behaving the same way. Bug: 831155, 826868 Change-Id: I468a24c31142cb0a8d646cd09928f7b03680d779 Reviewed-on: https://chromium-review.googlesource.com/c/1350874 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#612152} [modify] https://crrev.com/b78c35086d661489c42042e94362294cf3e9a15b/content/common/mime_sniffing_throttle_unittest.cc [modify] https://crrev.com/b78c35086d661489c42042e94362294cf3e9a15b/content/common/mime_sniffing_url_loader.cc
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/63f924d605b141a7ab84606f7c954007ddf9fbbf commit 63f924d605b141a7ab84606f7c954007ddf9fbbf Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Wed Dec 05 08:45:26 2018 PrefetchURLLoader. Always send a response's body data pipe on URL load. Bug: 831155, 826868 Change-Id: I170cdfbacb8b1d6f56b868717de51aa91ec596b6 Reviewed-on: https://chromium-review.googlesource.com/c/1352774 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#613909} [modify] https://crrev.com/63f924d605b141a7ab84606f7c954007ddf9fbbf/content/browser/loader/prefetch_url_loader.cc
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0bc656bf1d60b779bda6503f8852a8e0964725d5 commit 0bc656bf1d60b779bda6503f8852a8e0964725d5 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Wed Dec 05 11:15:39 2018 DCHECK: Successful URL load requires a response's body DCHECK that when URLLoaderClient::OnComplete(net::OK) is called, a response's body has been received before. Bug: 831155, 826868 Change-Id: I821267f5fdefcc1e8f18be6a96138237207e7b77 Reviewed-on: https://chromium-review.googlesource.com/c/1352259 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#613936} [modify] https://crrev.com/0bc656bf1d60b779bda6503f8852a8e0964725d5/content/renderer/loader/url_loader_client_impl.cc [modify] https://crrev.com/0bc656bf1d60b779bda6503f8852a8e0964725d5/content/renderer/loader/url_loader_client_impl.h [modify] https://crrev.com/0bc656bf1d60b779bda6503f8852a8e0964725d5/content/renderer/loader/url_loader_client_impl_unittest.cc
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a7fe6dfa93848704dd2eff05fe40656c6254328d commit a7fe6dfa93848704dd2eff05fe40656c6254328d Author: Jun Cai <juncai@chromium.org> Date: Wed Dec 05 20:38:50 2018 Revert "DCHECK: Successful URL load requires a response's body" This reverts commit 0bc656bf1d60b779bda6503f8852a8e0964725d5. Reason for revert: During network service sheriffing, noticed that the Mojo Linux bot started failing from: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mojo%20Linux/23165 and two layout tests failed: http/tests/worklet/import-filesystem-url.html virtual/threaded/http/tests/worklet/import-filesystem-url.html It looks like it is related to this CL. Original change's description: > DCHECK: Successful URL load requires a response's body > > DCHECK that when URLLoaderClient::OnComplete(net::OK) is called, a > response's body has been received before. > > Bug: 831155, 826868 > Change-Id: I821267f5fdefcc1e8f18be6a96138237207e7b77 > Reviewed-on: https://chromium-review.googlesource.com/c/1352259 > Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> > Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> > Cr-Commit-Position: refs/heads/master@{#613936} TBR=kinuko@chromium.org,arthursonzogni@chromium.org, jam@chromium.org Change-Id: I61cb591566839e6587390de82e90d8ccf08edfdd No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 831155, 826868 , 912293 Reviewed-on: https://chromium-review.googlesource.com/c/1363899 Reviewed-by: Jun Cai <juncai@chromium.org> Commit-Queue: Jun Cai <juncai@chromium.org> Cr-Commit-Position: refs/heads/master@{#614085} [modify] https://crrev.com/a7fe6dfa93848704dd2eff05fe40656c6254328d/content/renderer/loader/url_loader_client_impl.cc [modify] https://crrev.com/a7fe6dfa93848704dd2eff05fe40656c6254328d/content/renderer/loader/url_loader_client_impl.h [modify] https://crrev.com/a7fe6dfa93848704dd2eff05fe40656c6254328d/content/renderer/loader/url_loader_client_impl_unittest.cc
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/93f4cecd7900f827c4eab321b64072650a92ef6d commit 93f4cecd7900f827c4eab321b64072650a92ef6d Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Fri Dec 07 08:27:16 2018 FileSystemURLLoader: always send response's body. When a file is empty, the FileSystemURLLoader must send the response's head, but also the response's body. The latter was missing. Bug: 831155, 826868 , 912293 Change-Id: I51805ca596bd7d97e04221f24222e15576a20d34 Reviewed-on: https://chromium-review.googlesource.com/c/1365431 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#614641} [modify] https://crrev.com/93f4cecd7900f827c4eab321b64072650a92ef6d/content/browser/fileapi/file_system_url_loader_factory.cc
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a727d4b76ffcdf8c3dc03e19a08c90d217420c62 commit a727d4b76ffcdf8c3dc03e19a08c90d217420c62 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Mon Dec 10 09:10:18 2018 (reland) DCHECK: Successful URL load requires a response's body Reland of https://chromium-review.googlesource.com/c/chromium/src/+/1352259 DCHECK that when URLLoaderClient::OnComplete(net::OK) is called, a response's body has been received before. Bug: 831155, 826868 , 912293 Change-Id: I387fc3930cf3ffadbd6a8ddb6b96c30c4dbc1120 Reviewed-on: https://chromium-review.googlesource.com/c/1352259 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613936} Reviewed-on: https://chromium-review.googlesource.com/c/1365234 Cr-Commit-Position: refs/heads/master@{#615069} [modify] https://crrev.com/a727d4b76ffcdf8c3dc03e19a08c90d217420c62/content/renderer/loader/url_loader_client_impl.cc [modify] https://crrev.com/a727d4b76ffcdf8c3dc03e19a08c90d217420c62/content/renderer/loader/url_loader_client_impl.h [modify] https://crrev.com/a727d4b76ffcdf8c3dc03e19a08c90d217420c62/content/renderer/loader/url_loader_client_impl_unittest.cc
,
Dec 19
,
Jan 15
Hi Arthur, At issue 894819, We (shimazu@ and I) are working to start using mojo data pipe in the blink loading pipeline. In short, I want to replace void blink::ResourceLoader::DidReceiveResponse( const WebURLResponse&, std::unique_ptr<WebDataConsumerHandle> body) { ... } with void blink::ResourceLoader::DidReceiveResponse( const WebURLResponse&, mojo::DataPipeConsumerHandle body) { ... } . Currently WebDataConsumerHandle makes it possible to dispatch the response and body in the same call but we cannot do it with mojo data pipe. We can, of course, modify blink, but I want to know the status of this issue before making changes. If you are going to merge URLLoader::OnReceiveResponse() and URLLoader::OnStartLoadingResponseBody() soon, I will wait for that. Thanks!
,
Jan 15
Yes, merging both is something I would like to have. Implementation is done here: https://chromium-review.googlesource.com/c/chromium/src/+/1172290 However, I had to deals with other bugs/projects/requests in the meantime and didn't push for the review. Now I need to rebase. Once it is landed. I will make a finch experiment to validate the performance improvement. There is also IsolatedCodeCaching that might hide the improvement, so I need to test with and without it. I wouldn't recommend you to wait for me. It will take a few weeks.
,
Jan 16
Thank you for the information! |
||||
►
Sign in to add a comment |
||||
Comment 1 by arthurso...@chromium.org
, Apr 10 2018