Introduce PageSchemeRegistry
Created attachment 294202 [details] Patch
Attachment 294202 [details] did not pass style-queue: ERROR: Source/WebCore/page/PageSchemeRegistry.h:37: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 2 in 222 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 294203 [details] Patch
Comment on attachment 294203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294203&action=review > Source/WebCore/ChangeLog:8 > + No change in behavior. Just introduced a stub. This description is insufficient. Neither the description for this bug nor any of the ChangeLog files describe what this change is about and its motivation. Although this change is introducing a stub it is a step towards a larger architecture change and we should describe in these ChangeLog files what is PageSchemeRegistry, who creates/owns it, and how other fundamental classes (e.g. Document) relate to it or make use of it. We should also describe the motivation for this change, including why SchemeRegistry no longer works well in the post-WebKit2/Network process world. > Source/WebCore/dom/Document.h:1305 > + Document(Frame*, const URL&, const PageSchemeRegistry&, unsigned = DefaultDocumentClass, unsigned constructionFlags = 0); Can you please elaborate on this? In particular, can you please elaborate on the lifetime of the Document's PageSchemeRegistry with respect to the lifetime of the Frame object associated with the Document. > Source/WebCore/loader/DocumentLoader.cpp:1137 > m_archive = archive; We are underutilizing the purpose of passing by rvalue reference to remove unnecessary ref count churn. This will cause ref count churn. We should WTFMove() archive into m_archive. > Source/WebCore/loader/FrameLoader.cpp:894 > +void FrameLoader::loadArchive(RefPtr<Archive>&& archive) We should WTFMove() archive into Document::setArchive() below. Otherwise, we are are underutilizing the properties of passing by rvalue reference. > Source/WebCore/page/SecurityOrigin.cpp:273 > +bool SecurityOrigin::canRequest(const URL& url, const PageSchemeRegistry& schemeRegistry) const How did you come to the decision to pass a scheme registry to the SecurityOrigin when invoking a member function as opposed to using the scheme registry that was used to instantiate the SecurityOrigin? Are there cases where we want to pass the scheme registry for a document B when querying the security origin of document A? (Maybe this is answered if I keep reading through this patch?) > Source/WebCore/page/SecurityOrigin.cpp:328 > +bool SecurityOrigin::canDisplay(const URL& url, const PageSchemeRegistry& schemeRegistry) const Ditto.