New issue
Advanced search Search tips

Issue 695279 link

Starred by 6 users
Status: Archived
Owner:
kinuko@chromium.org
Closed: Sep 2017
Cc:
japhet@chromium.org
nhiroki@chromium.org
slangley@chromium.org
haraken@chromium.org
dcheng@chromium.org
horo@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 694904
Blocking:
issue 691794



Sign in to add a comment

Decouple loading and frame code

Project Member Reported by kinuko@chromium.org, Feb 23 2017 
Loading code is tightly coupled with Frame code, including the architecture for notifications / callbacks to outside Blink (via Frame -> FrameLoaderClientImpl -> WebFrameClient, or WebFrame -> WebFrameClient)

(More clarification: bunch of navigation related code is expected to go away after PlzNavigate, but we also have some plumbing for regular resource loading, which is not necessarily tied up to a frame)

These should be separated out from Frame as this architecture is making various Loading, Worker related optimization (e.g. off-MT loading) and refactoring very hard, as well as making Frame refactoring hard.

Related uber bug for Frame-side cleanup: issue 691794

Comment 1 by kinuko@chromium.org, Feb 23 2017

Cc: horo@chromium.org
Some of the plumbing I'm thinking about to decouple (some are SW-specific):

> dispatchWillSendRequeset
> dispatchDidLoadResourceFromMemoryCache
> isControlledByServiceWorker
> serviceWorkerID
> dispatchDidReceiveResponse

Comment 2 by kinuko@chromium.org, Feb 27 2017

Cc: nhiroki@chromium.org japhet@chromium.org
Also:

- didObserveLoadingBehavior
- dispatchDidCommitLoad (need to see how the PlzNavigate change affects)

Called via DocumentLoader -> FrameLoader::receivedFirstData path
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 2 2017 

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

commit 1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8
Author: kinuko <kinuko@chromium.org>
Date: Thu Mar 02 07:01:29 2017

Introduce ThreadableLoadingContext: make threaded loading code one step away from Document.

For now it's just a wrapper of Document.

BUG= 695279 , 538751

Review-Url: https://codereview.chromium.org/2715803004
Cr-Commit-Position: refs/heads/master@{#454198}

[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/loader/BUILD.gn
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/loader/ThreadableLoader.cpp
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp
[add] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/loader/ThreadableLoadingContext.cpp
[add] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/loader/ThreadableLoadingContext.h
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.h
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.cpp
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.h
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/workers/WorkerLoaderProxy.cpp
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/workers/WorkerLoaderProxy.h
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/core/workers/WorkerThreadTestHelper.h
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.h
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/modules/websockets/WebSocketChannel.h
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.h
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/web/WebAssociatedURLLoaderImpl.cpp
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.h
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp
[modify] https://crrev.com/1a7f9ee8e1a0d5944b59e4474c1344b8bf7acbf8/third_party/WebKit/Source/web/WebSharedWorkerImpl.h
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 23 2017 

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

commit 059d59531760005f8bfa5a03eab31fb9e3203ff5
Author: kinuko <kinuko@chromium.org>
Date: Thu Mar 23 07:42:31 2017

Make prepareRequest() a separate callback of FetchContext

- Split prepareRequest() from dispatchWillSendRequest() and willStartLoading()
- Move AppCache hook code into prepareRequest(), which looks more relevant
- Rename willStartLoading() to recordLoadingActivity() and makes it take const request

This still doesn't make dispatchWillSendRequest() take
const ResourceRequest& (see crbug.com/632580), but (I think) makes things
slightly more consistent & moves fetching logic out of context into fetcher
abstraction implementors like ResourceFetcher.

BUG= 695279 , 632580

Review-Url: https://codereview.chromium.org/2747203002
Cr-Commit-Position: refs/heads/master@{#459016}

[modify] https://crrev.com/059d59531760005f8bfa5a03eab31fb9e3203ff5/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/059d59531760005f8bfa5a03eab31fb9e3203ff5/third_party/WebKit/Source/core/loader/FrameFetchContext.h
[modify] https://crrev.com/059d59531760005f8bfa5a03eab31fb9e3203ff5/third_party/WebKit/Source/core/loader/PingLoader.cpp
[modify] https://crrev.com/059d59531760005f8bfa5a03eab31fb9e3203ff5/third_party/WebKit/Source/core/loader/appcache/ApplicationCacheHost.cpp
[modify] https://crrev.com/059d59531760005f8bfa5a03eab31fb9e3203ff5/third_party/WebKit/Source/core/loader/appcache/ApplicationCacheHost.h
[modify] https://crrev.com/059d59531760005f8bfa5a03eab31fb9e3203ff5/third_party/WebKit/Source/platform/loader/fetch/FetchContext.cpp
[modify] https://crrev.com/059d59531760005f8bfa5a03eab31fb9e3203ff5/third_party/WebKit/Source/platform/loader/fetch/FetchContext.h
[modify] https://crrev.com/059d59531760005f8bfa5a03eab31fb9e3203ff5/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/059d59531760005f8bfa5a03eab31fb9e3203ff5/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 30 2017 

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

commit 918293741a54580488930933602b2d4984378b97
Author: kinuko <kinuko@chromium.org>
Date: Thu Mar 30 05:54:27 2017

Move FrameFetchContext::dispatchDidLoadResourceFromMemoryCache logic into ResourceFetcher

Motivation: want to make FrameFetchContext thinner that only has the
code that hides Frame abstraction

BUG= 695279 

Review-Url: https://codereview.chromium.org/2751143003
Cr-Commit-Position: refs/heads/master@{#460676}

[modify] https://crrev.com/918293741a54580488930933602b2d4984378b97/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/918293741a54580488930933602b2d4984378b97/third_party/WebKit/Source/core/loader/FrameFetchContext.h
[modify] https://crrev.com/918293741a54580488930933602b2d4984378b97/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp
[modify] https://crrev.com/918293741a54580488930933602b2d4984378b97/third_party/WebKit/Source/platform/loader/fetch/FetchContext.cpp
[modify] https://crrev.com/918293741a54580488930933602b2d4984378b97/third_party/WebKit/Source/platform/loader/fetch/FetchContext.h
[modify] https://crrev.com/918293741a54580488930933602b2d4984378b97/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/918293741a54580488930933602b2d4984378b97/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.h
[modify] https://crrev.com/918293741a54580488930933602b2d4984378b97/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 5 2017 

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

commit 8bd5d6a15f00ab291c675c2e49b19275d263b6bc
Author: kinuko <kinuko@chromium.org>
Date: Wed Apr 05 09:54:59 2017

Separate ContentSettingsClient out from LocalFrameClient

LocalFrameClient is too overloaded, which is making it hard to figure out frame
dependency.  This CL tries to split ContentSettingsClient part out of LocalFrameClient
to clarify the dependency.  In this way it becomes possible to provide/implement
ContentSettingsClient without Frame.

This change also moves WebContentSettingsClient from public/web to public/platform
for easier layering, but if anyone disagrees I can revert the part.

BUG= 695279 
TBR=jochen@chromium.org, boliu@chromium.org

Review-Url: https://codereview.chromium.org/2786673002
Cr-Commit-Position: refs/heads/master@{#462020}

[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/android_webview/renderer/aw_content_settings_client.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/chrome/renderer/content_settings_observer.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/content/shell/test_runner/mock_content_settings_client.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/dom/BUILD.gn
[rename] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/dom/ContextFeaturesClientImpl.cpp
[rename] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/dom/ContextFeaturesClientImpl.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/frame/BUILD.gn
[add] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/frame/ContentSettingsClient.cpp
[add] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/frame/ContentSettingsClient.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/frame/LocalFrame.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/frame/LocalFrameClient.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/html/ImageDocument.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/loader/EmptyClients.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/loader/FrameFetchContext.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/core/loader/MixedContentChecker.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/modules/webdatabase/DatabaseClient.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/modules/webdatabase/DatabaseClient.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/BUILD.gn
[delete] https://crrev.com/19a2c47d9717b8f83d8ad79e914bd972eb1c1311/third_party/WebKit/Source/web/DatabaseClientImpl.cpp
[delete] https://crrev.com/19a2c47d9717b8f83d8ad79e914bd972eb1c1311/third_party/WebKit/Source/web/DatabaseClientImpl.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/DedicatedWorkerMessagingProxyProviderImpl.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/EditorClientImpl.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/IndexedDBClientImpl.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/LocalFileSystemClient.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/LocalFrameClientImpl.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/StorageClientImpl.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/public/BUILD.gn
[rename] https://crrev.com/8bd5d6a15f00ab291c675c2e49b19275d263b6bc/third_party/WebKit/public/platform/WebContentSettingsClient.h
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 7 2017 

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

commit f8712b46e6a764fa43e1e06b31d026c89a8f196e
Author: kinuko <kinuko@chromium.org>
Date: Fri Apr 07 10:24:52 2017

Remove FrameFetchContext::dispatchDidReceiveResponseInternal declaration

The implentation was removed sometime ago, but I forgot to remove
the declaration.

BUG= 695279 
R=horo@chromium.org,japhet@chromium.org

Review-Url: https://codereview.chromium.org/2797423004
Cr-Commit-Position: refs/heads/master@{#462828}

[modify] https://crrev.com/f8712b46e6a764fa43e1e06b31d026c89a8f196e/third_party/WebKit/Source/core/loader/FrameFetchContext.h
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 24 2017 

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

commit c9dd99240fda60e9daca1f9b87c20a92b10de38a
Author: kinuko <kinuko@chromium.org>
Date: Mon Apr 24 09:30:36 2017

Implement CanRequest in BaseFetchContext

BUG= 695279 , 538751

Review-Url: https://codereview.chromium.org/2823213002
Cr-Commit-Position: refs/heads/master@{#466610}

[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.h
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/loader/BaseFetchContext.cpp
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/loader/BaseFetchContext.h
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/loader/BaseFetchContextTest.cpp
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/loader/FrameFetchContext.h
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/loader/SubresourceFilter.cpp
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/loader/SubresourceFilter.h
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/testing/NullExecutionContext.cpp
[modify] https://crrev.com/c9dd99240fda60e9daca1f9b87c20a92b10de38a/third_party/WebKit/Source/core/testing/NullExecutionContext.h

Comment 10 by kinuko@chromium.org, Jun 8 2017

Blocking: -443374

Comment 11 by kinuko@chromium.org, Sep 29 2017

Status: Archived (was: Assigned)
There're several related re-archi work, and this work's going to be subsumed by:
- Off-main-thread fetch ( issue 443374 ) --> fixed
- Off-main-thread fetch for Sync ( issue 706331 ) --> to be picked up
- Per-frame loading code ( issue 712913 ) --> being worked on
- Remove shadow document (issue 538751) --> will resume after  issue 706331  is done

Given that I'm closing this one.

Comment 12 by kinuko@chromium.org, Sep 29 2017

Blocking: -538751
►

Sign in to add a comment