New issue
Advanced search Search tips

Issue 808759 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug
Proj-Servicification

Blocked on:
issue 800901



Sign in to add a comment

DownloadContentTest.DownloadAttributeBlobURL with Network Service enabled

Project Member Reported by reillyg@chromium.org, Feb 3 2018

Issue description

https://chromium-review.googlesource.com/899666 was reverted because it was causing DownloadContentTest.DownloadAttributeBlobURL to fail on Linux Tests, Linux Tests (dbg)(1), and Linux Tests (dbg)(1)(32).
https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests/66991
https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29%2832%29/47823
https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29/70044

This issue tracks resolving that issue.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a5561649b9567c394ef1ff1619745b02ed7759d

commit 8a5561649b9567c394ef1ff1619745b02ed7759d
Author: Reilly Grant <reillyg@chromium.org>
Date: Sat Feb 03 04:37:45 2018

Reland "Bounce Blob URL revoking through the UI thread to give navigation a chance."

This is a reland of a3dd29b25f01eef7bf726aef971d554ed153a4f0.

Original change's description:
> Bounce Blob URL revoking through the UI thread to give navigation a chance.
>
> Navigation IPCs are handled on the UI thread so if a blob URL is revoked
> immediately after navigating the revocation will almost always happen before
> the navigation code has had a chance to post a task back to the IO thread to
> actually perform the request. With this change revocation is hopefully
> delayed just long enough to make navigation work again.
>
> Bug:  807639 
> Change-Id: I2afde86dd203c61cbb03cc02ac62ec21c7e7d704
> Reviewed-on: https://chromium-review.googlesource.com/899666
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#534119}

TBR=mek@chromium.org,jochen@chromium.org

Bug:  807639 , 808759 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I5033422d57ac7672e761045f7871c6aed29368b9
Reviewed-on: https://chromium-review.googlesource.com/900191
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534257}
[modify] https://crrev.com/8a5561649b9567c394ef1ff1619745b02ed7759d/content/browser/blob_storage/blob_dispatcher_host.cc
[modify] https://crrev.com/8a5561649b9567c394ef1ff1619745b02ed7759d/content/test/data/download/download-attribute-blob.html
[modify] https://crrev.com/8a5561649b9567c394ef1ff1619745b02ed7759d/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 5 2018

Labels: merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b43f737ce7b421ca8717f604db045e7fbb4e7b3

commit 7b43f737ce7b421ca8717f604db045e7fbb4e7b3
Author: Jochen Eisinger <jochen@chromium.org>
Date: Mon Feb 05 17:05:22 2018

Reland "Bounce Blob URL revoking through the UI thread to give navigation a chance."

This is a reland of a3dd29b25f01eef7bf726aef971d554ed153a4f0.

Original change's description:
> Bounce Blob URL revoking through the UI thread to give navigation a chance.
>
> Navigation IPCs are handled on the UI thread so if a blob URL is revoked
> immediately after navigating the revocation will almost always happen before
> the navigation code has had a chance to post a task back to the IO thread to
> actually perform the request. With this change revocation is hopefully
> delayed just long enough to make navigation work again.
>
> Bug:  807639 
> Change-Id: I2afde86dd203c61cbb03cc02ac62ec21c7e7d704
> Reviewed-on: https://chromium-review.googlesource.com/899666
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#534119}

TBR=jochen@chromium.org, mek@chromium.org, reillyg@chromium.org

(cherry picked from commit 8a5561649b9567c394ef1ff1619745b02ed7759d)

Bug:  807639 , 808759 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I5033422d57ac7672e761045f7871c6aed29368b9
Reviewed-on: https://chromium-review.googlesource.com/900191
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#534257}
Reviewed-on: https://chromium-review.googlesource.com/902043
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#302}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/7b43f737ce7b421ca8717f604db045e7fbb4e7b3/content/browser/blob_storage/blob_dispatcher_host.cc
[modify] https://crrev.com/7b43f737ce7b421ca8717f604db045e7fbb4e7b3/content/test/data/download/download-attribute-blob.html
[modify] https://crrev.com/7b43f737ce7b421ca8717f604db045e7fbb4e7b3/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Comment 3 by mek@chromium.org, Feb 5 2018

Blockedon: 800901
Components: Blink>Storage
Status: Started (was: Assigned)
As more background, that test has never worked with network service. It just was unfortunately temporarily removed. But anyway, the work I'm doing on issue 800901 fixes this.

Comment 4 by jam@chromium.org, Apr 25 2018

@Marijn: looks like that test works now, so can it be enabled?

Comment 5 by mek@chromium.org, Apr 25 2018

Huh? I wouldn't expect it to work yet? Except perhaps flakily. https://chromium-review.googlesource.com/c/chromium/src/+/1026808 is supposed to fix it.

Comment 6 by jam@chromium.org, Apr 25 2018

ah, ok maybe it's flaky :)
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4d4aa99b532b9f5685316b0fa85fd6ca50667216

commit 4d4aa99b532b9f5685316b0fa85fd6ca50667216
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Fri Apr 27 18:03:27 2018

[Mojo Blob URLs] Handle downloads.

This changes the DownloadURL IPC to pass along a blob URL token, and
uses that to later construct a URLLoaderFactory for the blob URL.

Only works with the network service enabled, but at least fixes ordering
issues already present with the network service and downloading soon to
be revoked blob URLs.

Eventually this should let us remove the other blob specific codepaths
from (at least the network service branch of) the downloads code.

Design doc: https://docs.google.com/document/d/1DqVcTWE9Qb_3KpIRH2bFV-6hWEr8S92c4ppY67YL1KI/edit#heading=h.pvtybbkgf0vd

Bug: 800901,  808759 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I5879fdc65f4843a1296a49496fc73ed6c3a04be1
Reviewed-on: https://chromium-review.googlesource.com/1026808
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554418}
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/components/download/content/internal/download_driver_impl.cc
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/browser/BUILD.gn
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/browser/bad_message.h
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/browser/download/download_manager_impl.h
[add] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/browser/download/download_url_loader_factory_getter_impl.cc
[add] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/browser/download/download_url_loader_factory_getter_impl.h
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/browser/frame_host/render_frame_message_filter.cc
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/browser/frame_host/render_frame_message_filter.h
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/browser/renderer_host/render_view_host_unittest.cc
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/common/frame_messages.h
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/public/browser/download_manager.h
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/public/test/mock_download_manager.h
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/renderer/render_frame_impl.h
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/shell/test_runner/web_frame_test_client.cc
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/shell/test_runner/web_frame_test_client.h
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/content/shell/test_runner/web_frame_test_proxy.h
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/third_party/blink/public/web/web_frame_client.h
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/third_party/blink/renderer/core/exported/local_frame_client_impl.cc
[modify] https://crrev.com/4d4aa99b532b9f5685316b0fa85fd6ca50667216/tools/metrics/histograms/enums.xml

Comment 8 by mek@chromium.org, Apr 27 2018

Status: Fixed (was: Started)

Sign in to add a comment