New issue
Advanced search Search tips
Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 918967



Sign in to add a comment
link

Issue 891872: Remove blink::Platform::CreateDefaultURLLoaderFactory and its callers

Reported by lukasza@chromium.org, Oct 3 Project Member

Issue description

The URLLoaderFactory created by blink::Platform::CreateDefaultURLLoaderFactory is not associated with a specific origin.  This is undesirable, because it makes it forces such URLLoaderFactory to trust the network::ResourceRequest::request_initiator provided by the factory (also note that the NetworkService doesn't have knowledge about origin locks that content::ChildProcessSecurityPolicyImpl might have applies to the given renderer process - associating the URLLoaderFactory with a given origin has to happen in the browser process, when creating the factory).

Let's use this bug to track removal of blink::Platform::CreateDefaultURLLoaderFactory.
 

Comment 1 by lukasza@chromium.org, Oct 3

Blocking: 871827

Comment 2 by lukasza@chromium.org, Oct 3

I've just found issue 751425, but I assume that this is a separate bug.

Comment 3 by lukasza@chromium.org, Oct 3

- WIP CL @ https://crrev.com/c/1259824 tries to remove the caller in WebViewPlugin::WebViewHelper::CreateURLLoaderFactory

- WIP CL @ https://crrev.com/c/1259613 changes the name of the method to
  CreateDeprecatedDefaultURLLoaderFactory
  (not sure if it is worth landing this CL, but it is helpful for showing
   how many callers+overrides there are)

Comment 4 by bugdroid1@chromium.org, Nov 16

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/905bf17717a301cb5c8c9b1ac417b40d0a97354e

commit 905bf17717a301cb5c8c9b1ac417b40d0a97354e
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Nov 16 02:14:37 2018

Return frame/origin-bound URLLoaderFactory from WebViewPlugin/Helper.

This CL modifies WebViewPlugin::WebViewHelper::CreateURLLoaderFactory
method so that it delegates creation of URLLoaderFactory to the frame
that hosts the plugin.

Bug: 891872
Change-Id: I002503730f98adbced92d61c7d74eb53f4865e98
Reviewed-on: https://chromium-review.googlesource.com/c/1259824
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608632}
[modify] https://crrev.com/905bf17717a301cb5c8c9b1ac417b40d0a97354e/components/plugins/renderer/webview_plugin.cc
[modify] https://crrev.com/905bf17717a301cb5c8c9b1ac417b40d0a97354e/third_party/blink/public/platform/platform.h

Comment 5 by dtapu...@chromium.org, Nov 27

Components: Blink>Loader

Comment 6 by yhirano@chromium.org, Nov 27

Cc: yhirano@chromium.org

Comment 7 by lukasza@chromium.org, Nov 29

FWIW, I've tried enumerating all the callers and implementations in a doc here: https://docs.google.com/document/d/115wW7v62cXQxdekdEpptmsDh9CeBy7xqyNJ6BAP_aF8

Feel free to chime-in and help chip-away at this bug :-)

Comment 8 by jam@chromium.org, Nov 30

Just checking: is this for the network service path only? afaik the default one is only used in some edge conditions there.

for the non-network service path, we use the global one. are you planning on changing that code path to send one per frame commit like the NS path (or perhaps ignoring that case for now?)?

Comment 9 by bugdroid1@chromium.org, Nov 30

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ff9445e99156f542f1c512515b9707b50169137

commit 9ff9445e99156f542f1c512515b9707b50169137
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Nov 30 19:37:17 2018

Use frame/origin-bound URLLoaderFactory in PrepareFrameAndViewForPrint.

This CL modifies PrepareFrameAndViewForPrint::CreateURLLoaderFactory
method so that it delegates creation of URLLoaderFactory to the frame
being printed.

Bug: 891872
Change-Id: I42a87f908e18b751bf8dd1d192b4fa2e34141d09
Reviewed-on: https://chromium-review.googlesource.com/c/1356022
Reviewed-by: Wei Li <weili@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612728}
[modify] https://crrev.com/9ff9445e99156f542f1c512515b9707b50169137/components/printing/renderer/print_render_frame_helper.cc

Comment 10 by lukasza@google.com, Nov 30

RE: jam@: #c8:

RE: Just checking: is this for the network service path only?

I am not sure if I understand the question (and how Blink works with/without NetworkService) to answer the above.  URLLoaderFactory that is not associated with a specific origin/frame is undesirable mostly (only?) in NetworkService path, because in NetworkService URLLoaderFactory should be able to get |request_initiator_origin_lock| information from the browser (to avoid trusting the renderer here.  With that perspective, presence of a global, non-frame/origin-associated blink::Platform::CreateDefaultURLLoaderFactory blocks issue 871827.
Yes.  Thisafaik the default one is only used in some edge conditions there.


RE: for the non-network service path, we use the global one. are you planning on changing that code path to send one per frame commit like the NS path (or perhaps ignoring that case for now?)?

I plan to ignore that case.

Comment 11 by toyoshim@chromium.org, Dec 3

Owner: lukasza@chromium.org
Status: Assigned (was: Untriaged)
Loading bug triage: let me make this assigned since lukasza seems to have worked on this.

Comment 12 by lukasza@chromium.org, Dec 3

Cc: falken@chromium.org f...@opera.com iclell...@chromium.org
+falken@ for WorkerShadowPage
+iclelland@ and fs@ for sandboxed subresource fetches made for SVG

I wonder if the remaining callers in the product code might be satisfied if the URL requests were made on behalf of an opaque/unique/sandboxed origin.

I see that SVGImage::DataChanged contains the following code:

      LocalFrame* frame = nullptr;
      {
        TRACE_EVENT0("blink", "SVGImage::dataChanged::createFrame");
        DCHECK(!frame_client_);
        frame_client_ = MakeGarbageCollected<SVGImageLocalFrameClient>(this);
        frame = LocalFrame::Create(frame_client_, *page, nullptr);
        frame->SetView(LocalFrameView::Create(*frame));
        frame->Init();
      }

      FrameLoader& loader = frame->Loader();
      loader.ForceSandboxFlags(kSandboxAll);  // <--- *** SANDBOXING FORCED HERE ***

I also strongly suspect that WebPagePopupImpl, InspectorOverlayAgent, ValidationMessageOverlayDelegate either don't make requests at all (if so, https://crrev.com/c/1359051 would address them), or would be satisfied if the requests were made on behalf of an opaque/unique/sandboxed origin.

I am not so sure about WorkerShadowPage (whether it is okay to use opaque origin for subresource fetches OR if it might be possible to use a URLLoaderFactory associated with a specific, non-opaque origin instead)...

Comment 13 by falken@chromium.org, Dec 4

We're trying to remove WorkerShadowPage completely. Let me look into why this callsite is needed.

Comment 14 by falken@chromium.org, Dec 5

As I just wrote on the doc, the callsite in WorkerShadowPage is only needed in the non-ServiceWorkerServicification case, as far as I can tell.

I'm wondering if blink::Platform::CreateDefaultURLLoaderFactoryBundle has the same problem as blink::Platform::CreateDefaultURLLoaderFactory? I can make service worker behave like shared worker, which would remove the CreateDefaultURLLoaderFactory call, but at the expense of adding CreateDefaultURLLoaderFactoryBundle. See:
https://cs.chromium.org/chromium/src/content/renderer/shared_worker/embedded_shared_worker_stub.cc?l=270&rcl=66ed852e522a88a88ec863099ecf1565bcf1ab9e

Comment 15 by falken@chromium.org, Dec 5

CreateDefaultURLLoaderFactoryBundle is also called for frames (the shared worker code largely copy/pasted that):
https://cs.chromium.org/chromium/src/content/renderer/render_frame_impl.cc?l=6532&rcl=9bc27b1de4b78e524936c7e4327128bc0bc18843

If we don't care about the non-ServiceWorkerServicification path (since you don't care about the non-NetworkService path, if I'm reading c#10 correctly), another option is to put CHECK(!ServiceWorkerServicification) at the callsite, and then remove it once the non-ServiceWorkerServicification code is removed.

Comment 16 by bugdroid1@chromium.org, Dec 5

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/95065abbd18c9be7661f8f64929ef17624090f64

commit 95065abbd18c9be7661f8f64929ef17624090f64
Author: Matt Falkenhagen <falken@chromium.org>
Date: Wed Dec 05 03:03:16 2018

service worker: Use fake network provider in WebEmbeddedWorkerImplTest.

WebEmbeddedWorkerImplTest was using a null ServiceWorkerNetworkProvider.
This CL adds a fake provider which implements CreateURLLoader() to
return non-null, which matches the ServiceWorkerServicification
behavior.  This removes callsites to
Platform::CreateDefaultURLLoaderFactory() via
WorkerShadowPage::CreateURLLoaderFactory(), which is being removed at
bug 891872.

Bug: 891872
Change-Id: I8592131fea83429b39b9744727a4bdd493734be0
Reviewed-on: https://chromium-review.googlesource.com/c/1361754
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613836}
[modify] https://crrev.com/95065abbd18c9be7661f8f64929ef17624090f64/third_party/blink/renderer/modules/service_worker/web_embedded_worker_impl_test.cc

Comment 17 by lukasza@chromium.org, Dec 5

falken@: Thank you very much for looking into WorkerShadowPage and how it relates to this bug - much appreciated!


RE: I'm wondering if blink::Platform::CreateDefaultURLLoaderFactoryBundle has the same problem as blink::Platform::CreateDefaultURLLoaderFactory?

I am not sure what you mean by blink::Platform::CreateDefaultURLLoaderFactoryBundle.  I could not find that method in Chromium code search.  I assume that you mean content::RendererBlinkPlatformImpl::CreateDefaultURLLoaderFactoryBundle.

blink::Platform::CreateDefaultURLLoaderFactory is problematic, because the URLLoaderFactory it returns is *not* associated with a specific frame and/or origin.  This problem is also present in content::RendererBlinkPlatformImpl::CreateDefaultURLLoaderFactoryBundle and RendererBlinkPlatformImpl::CreateNetworkURLLoaderFactory - all of these methods eventually result in a mojo IPC that asks RenderProcessHostImpl for a URLLoaderFactory (via blink::mojom::AssociatedInterfaceProvider).

It is preferable for renderer/Blink features to obtain URLLoaderFactory via a frame or worker context instead (e.g. by calling local_frame->Client()->CreateURLLoaderFactory() instead;  see also r612728 and r608632 above).  Obtaining URLLoaderFactory in such way means that the browser process can associate the factory with a specific frame and/or origin (i.e. the browser process creates such factory via RenderFrameHostImpl::CreateNetworkServiceDefaultFactoryInternal called at RenderFrameHostImpl::CommitNavigation time) and this in turn means that we don't need to trust |request_initiator| coming from the renderer (see issue 871827).


RE: WorkerShadowPage is only needed in the non-ServiceWorkerServicification case, as far as I can tell.

I am not sure what you mean by ServiceWorkerServicification.  I assume that you mean --enable-features=NetworkService.  If this assumption is correct, then I think the problematic case is hit even with --enable-features=NetworkService.  For example, patch-in https://crrev.com/c/1363638 (at ToT / r613994 / after the r613836 from #c18 above) and try running external/wpt/service-workers/service-worker/referer.https.html with --enable-features=NetworkService - the test will crash, because of the following callstack:

    #2 content::RendererBlinkPlatformImpl::CreateDefaultURLLoaderFactoryBundle()
    #3 content::RenderFrameImpl::SetupLoaderFactoryBundle()
    #4 content::RenderFrameImpl::GetLoaderFactoryBundle()
    #5 content::RenderFrameImpl::BuildServiceWorkerNetworkProviderForNavigation()
    #6 content::RenderFrameImpl::DidCreateDocumentLoader()
    #7 blink::LocalFrameClientImpl::CreateDocumentLoader()
    #8 blink::FrameLoader::CreateDocumentLoader()
    #9 blink::FrameLoader::Init()
    #10 blink::LocalFrame::Init()
    #11 blink::WebLocalFrameImpl::InitializeCoreFrame()
    #12 blink::WebLocalFrameImpl::CreateMainFrame()
    #13 content::RenderFrameImpl::CreateMainFrame()
    #14 content::RenderViewImpl::Initialize()
    #15 content::RenderViewImpl::Create()
    #16 content::RenderThreadImpl::CreateView()

I do note that the callstack above doesn't go through blink::Platform::CreateDefaultURLLoaderFactory, but since this is basically the same problem, I think opening a separate bug is not desirable / necessary.

Comment 18 by bugdroid1@chromium.org, Dec 5

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/363b61e5a8dfd4949ffa369f7e0243ac0e6afb04

commit 363b61e5a8dfd4949ffa369f7e0243ac0e6afb04
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Wed Dec 05 23:22:29 2018

Make EmptyLocalFrameClient disallow network requests by default.

As we attempt to limit the uses of blink::Platform’s
CreateDefaultURLLoaderFactory, it makes sense to avoid using it from
EmptyLocalFrameClient - this CL disallows the default
EmptyLocalFrameClient from making network requests.

This CL assumes that WebPagePopupImpl, InspectorOverlayAgent,
ValidationMessageOverlayDelegate should not make any network requests.
After this CL, these classes will continue using EmptyLocalFrameClient
and if they attempt to make fetch a subresource over network, then they
will hit the new NOTREACHED (and probably crash shortly after when
dereferencing the returned, null URLLoaderFactory).

This CL preserves the behavior of SVGImage::SVGImageLocalFrameClient,
DummyPageHolder (test code) and PingLocalFrameClient (test code)
to continue calling blink::Platform's CreateDefaultURLLoaderFactory.

Bug: 891872
Change-Id: I5132fcc66929ff14b25ce2ce636011e1463244f4
Reviewed-on: https://chromium-review.googlesource.com/c/1359051
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614168}
[modify] https://crrev.com/363b61e5a8dfd4949ffa369f7e0243ac0e6afb04/third_party/blink/renderer/core/loader/empty_clients.h
[modify] https://crrev.com/363b61e5a8dfd4949ffa369f7e0243ac0e6afb04/third_party/blink/renderer/core/loader/ping_loader_test.cc
[modify] https://crrev.com/363b61e5a8dfd4949ffa369f7e0243ac0e6afb04/third_party/blink/renderer/core/svg/graphics/svg_image.cc
[modify] https://crrev.com/363b61e5a8dfd4949ffa369f7e0243ac0e6afb04/third_party/blink/renderer/core/testing/dummy_page_holder.cc

Comment 19 by falken@chromium.org, Dec 6

c#17:

> I am not sure what you mean by blink::Platform::CreateDefaultURLLoaderFactoryBundle.  I could not find that method in Chromium code search.  I assume that you mean content::RendererBlinkPlatformImpl::CreateDefaultURLLoaderFactoryBundle.

Sorry yes, that's right.

> I am not sure what you mean by ServiceWorkerServicification.  I assume that you mean --enable-features=NetworkService.

It's --enable-features=ServiceWorkerServicification. But when NetworkService is enabled, it effectively enables SWS as well. It's a subset of NS that can be shipped separately.

> For example, patch-in https://crrev.com/c/1363638 (at ToT / r613994 / after the r613836 from #c18 above) 

Yes I see, but even if you patch that in and remove the callsite from SWNetworkProviderForNavigation code (just use nullptr for |fallback_factory|), we still call it in CommitNavigation:

#7 0x7ffb2a00914f content::RendererBlinkPlatformImpl::CreateDefaultURLLoaderFactoryBundle()
#8 0x7ffb29fb8547 content::RenderFrameImpl::SetupLoaderFactoryBundle()
#9 0x7ffb29fb776a content::RenderFrameImpl::CommitNavigation()

That's why I was wondering whether it's OK to call CreateDefaultURLLoaderFactoryBundle() for service worker startup. I understand now it's not, and frames are doing the same thing.

I can look more into removing it from service workers, but we'll need a solution to remove it from frames as well.

Comment 20 by bugdroid1@chromium.org, Dec 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f2252e6d6622f13d03f20e613fcaa13c34cc5b4

commit 3f2252e6d6622f13d03f20e613fcaa13c34cc5b4
Author: Łukasz Anforowicz <lukasza@chromium.org>
Date: Sat Dec 08 00:13:51 2018

Revert "Use frame/origin-bound URLLoaderFactory in PrepareFrameAndViewForPrint."

This reverts commit 9ff9445e99156f542f1c512515b9707b50169137.

Reason for revert: Main suspect for causing https://crbug.com/911250

Original change's description:
> Use frame/origin-bound URLLoaderFactory in PrepareFrameAndViewForPrint.
> 
> This CL modifies PrepareFrameAndViewForPrint::CreateURLLoaderFactory
> method so that it delegates creation of URLLoaderFactory to the frame
> being printed.
> 
> Bug: 891872
> Change-Id: I42a87f908e18b751bf8dd1d192b4fa2e34141d09
> Reviewed-on: https://chromium-review.googlesource.com/c/1356022
> Reviewed-by: Wei Li <weili@chromium.org>
> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612728}

TBR=weili@chromium.org,lukasza@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 891872
Change-Id: I7bfac93ff70c09b8ef22ca4225b2216076e7fc1b
Reviewed-on: https://chromium-review.googlesource.com/c/1368781
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614878}
[modify] https://crrev.com/3f2252e6d6622f13d03f20e613fcaa13c34cc5b4/components/printing/renderer/print_render_frame_helper.cc

Comment 21 by kinuko@chromium.org, Dec 10

Summary: Remove blink::Platform::CreateDefaultURLLoaderFactoryBundle and its callers (was: Remove blink::Platform::CreateDefaultURLLoaderFactory and its callers)

Comment 22 by kinuko@chromium.org, Dec 10

Summary: Remove blink::Platform::CreateDefaultURLLoaderFactory and its callers (was: Remove blink::Platform::CreateDefaultURLLoaderFactoryBundle and its callers)

Comment 23 by bugdroid1@chromium.org, Dec 13

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f2cede3f8cdd694c865fc394a9af3edc672b10d

commit 9f2cede3f8cdd694c865fc394a9af3edc672b10d
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Thu Dec 13 11:31:36 2018

Stop using default_factory for AppCache and reduce RBPI::CreateDefaultURLLoaderFactoryBundle usage

* Stop using URLLoaderFactoryBundle::default_factory for AppCache,
  this over-generalization has been causing a lot of confusions.
* Deprecate URLLoaderFactoryBundle::default_network_factory, which is
  no longer needed after removing the default_factory == AppCache hack.
* This change also allows us to remove more RendererBlinkPlatformImpl::
  CreateDefaultURLLoaderFactoryBundle callsites for most of
  NetworkService-enabled cases, which was needed only when
  default_factory was dropped (just to skip AppCache, which is no
  longer needed), or for initial empty document cases (this is
  still needed).

Bug: 891872
Change-Id: I9b58097430b76d96d96864043b63553e88e3066c
Reviewed-on: https://chromium-review.googlesource.com/c/1371310
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616269}
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/browser/service_worker/embedded_worker_instance.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/browser/worker_host/shared_worker_host.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/browser/worker_host/shared_worker_host_unittest.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/common/navigation_subresource_loader_params.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/common/navigation_subresource_loader_params.h
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/common/url_loader_factory_bundle.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/common/url_loader_factory_bundle.h
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/common/url_loader_factory_bundle.mojom
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/common/url_loader_factory_bundle_struct_traits.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/common/url_loader_factory_bundle_struct_traits.h
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/renderer/loader/child_url_loader_factory_bundle.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/renderer/loader/child_url_loader_factory_bundle.h
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/renderer/loader/tracked_child_url_loader_factory_bundle.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/renderer/loader/tracked_child_url_loader_factory_bundle.h
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/9f2cede3f8cdd694c865fc394a9af3edc672b10d/content/renderer/shared_worker/embedded_shared_worker_stub.cc

Comment 24 by lukasza@chromium.org, Jan 3

Blocking: 918967

Comment 25 by lukasza@chromium.org, Jan 3

Blocking: -871827

Comment 26 by cr-audit...@appspot.gserviceaccount.com, Feb 19 (5 days ago)

Project Member
Labels: CommitLog-Audit-Violation Merge-Without-Approval M-72
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision ac0159d783ce12435615f91ef0f9afe7876a8828 was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required --

Comment 27 by cr-audit...@appspot.gserviceaccount.com, Feb 19 (5 days ago)

Project Member
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ac0159d783ce12435615f91ef0f9afe7876a8828

Commit: ac0159d783ce12435615f91ef0f9afe7876a8828
Author: kinuko@chromium.org
Commiter: jam@chromium.org
Date: 2019-02-19 01:27:55 +0000 UTC

Stop using default_factory for AppCache and reduce RBPI::CreateDefaultURLLoaderFactoryBundle usage

* Stop using URLLoaderFactoryBundle::default_factory for AppCache,
  this over-generalization has been causing a lot of confusions.
* Deprecate URLLoaderFactoryBundle::default_network_factory, which is
  no longer needed after removing the default_factory == AppCache hack.
* This change also allows us to remove more RendererBlinkPlatformImpl::
  CreateDefaultURLLoaderFactoryBundle callsites for most of
  NetworkService-enabled cases, which was needed only when
  default_factory was dropped (just to skip AppCache, which is no
  longer needed), or for initial empty document cases (this is
  still needed).

Bug: 891872
Change-Id: I9b58097430b76d96d96864043b63553e88e3066c
Reviewed-on: https://chromium-review.googlesource.com/c/1371310
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616269}(cherry picked from commit 9f2cede3f8cdd694c865fc394a9af3edc672b10d)
Reviewed-on: https://chromium-review.googlesource.com/c/1474539
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#873}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Comment 28 by bugdroid, Feb 19 (5 days ago)

Project Member
Labels: merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac0159d783ce12435615f91ef0f9afe7876a8828

commit ac0159d783ce12435615f91ef0f9afe7876a8828
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Tue Feb 19 01:27:55 2019

Stop using default_factory for AppCache and reduce RBPI::CreateDefaultURLLoaderFactoryBundle usage

* Stop using URLLoaderFactoryBundle::default_factory for AppCache,
  this over-generalization has been causing a lot of confusions.
* Deprecate URLLoaderFactoryBundle::default_network_factory, which is
  no longer needed after removing the default_factory == AppCache hack.
* This change also allows us to remove more RendererBlinkPlatformImpl::
  CreateDefaultURLLoaderFactoryBundle callsites for most of
  NetworkService-enabled cases, which was needed only when
  default_factory was dropped (just to skip AppCache, which is no
  longer needed), or for initial empty document cases (this is
  still needed).

Bug: 891872
Change-Id: I9b58097430b76d96d96864043b63553e88e3066c
Reviewed-on: https://chromium-review.googlesource.com/c/1371310
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616269}(cherry picked from commit 9f2cede3f8cdd694c865fc394a9af3edc672b10d)
Reviewed-on: https://chromium-review.googlesource.com/c/1474539
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#873}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/browser/service_worker/embedded_worker_instance.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/browser/worker_host/shared_worker_host.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/browser/worker_host/shared_worker_host_unittest.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/common/navigation_subresource_loader_params.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/common/navigation_subresource_loader_params.h
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/common/url_loader_factory_bundle.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/common/url_loader_factory_bundle.h
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/common/url_loader_factory_bundle.mojom
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/common/url_loader_factory_bundle_struct_traits.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/common/url_loader_factory_bundle_struct_traits.h
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/renderer/loader/child_url_loader_factory_bundle.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/renderer/loader/child_url_loader_factory_bundle.h
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/renderer/loader/tracked_child_url_loader_factory_bundle.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/renderer/loader/tracked_child_url_loader_factory_bundle.h
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/ac0159d783ce12435615f91ef0f9afe7876a8828/content/renderer/shared_worker/embedded_shared_worker_stub.cc

Comment 29 by jam@chromium.org, Feb 19 (5 days ago)

FYI the merge above to 72 is for  bug 931588  (I should have updated the bug id )

Sign in to add a comment