Don't create a new WebContent process when navigating to a blob object URL if the source document has access to it.
<rdar://problem/39715044>
Created attachment 338716 [details] Fixes the bug
Attachment 338716 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1057: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1063: Missing spaces around / [whitespace/operators] [3] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1066: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] Total errors found: 3 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 338716 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=338716&action=review > Source/WebCore/loader/NavigationAction.cpp:60 > + , m_isSameOriginBlobNavigation { isSameOriginBlogNavigation(source, resourceRequest.url()) } Should we have the same strategy with data URLs for performance reasons? Should we add a m_isConsideredAsSameOriginNavigation boolean flag taking care of all these cases (blob URLs, data URLs, about:blank, same origin URLs, file URLs...) ?
(In reply to youenn fablet from comment #4) > Comment on attachment 338716 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=338716&action=review > > > Source/WebCore/loader/NavigationAction.cpp:60 > > + , m_isSameOriginBlobNavigation { isSameOriginBlogNavigation(source, resourceRequest.url()) } > > Should we have the same strategy with data URLs for performance reasons? > Should we add a m_isConsideredAsSameOriginNavigation boolean flag taking > care of all these cases (blob URLs, data URLs, about:blank, same origin > URLs, file URLs...) ? That probably makes sense. Maybe m_shouldTreatAsSameOriginNavigation.
Created attachment 338766 [details] Made data: and about: use same process
Attachment 338766 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1057: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1063: Missing spaces around / [whitespace/operators] [3] ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1066: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] Total errors found: 3 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 338766 [details] Made data: and about: use same process View in context: https://bugs.webkit.org/attachment.cgi?id=338766&action=review > Source/WebCore/ChangeLog:9 > + to avoid creating a new WebContent process when navigating to a blob object URL. s/blob object// > Source/WebKit/UIProcess/WebPageProxy.cpp:3959 > + navigation->settreatAsSameOriginNavigation(navigationActionData.treatAsSameOriginNavigation); s/settreatAsSameOriginNavigation/setTreatAsSameOriginNavigation/ > Source/WebKit/UIProcess/WebProcessPool.cpp:2027 > + if (navigation.treatAsSameOriginNavigation()) Can we simplify the logic here with the lines below checking for !url.isValid() || url.isEmpty() || url.isBlankURL() || protocolHostAndPortAreEqual(url, targetURL)? Maybe WebProcess could do the computation here. I am wondering what we should do with file based origins/loads as well. > Source/WebKit/UIProcess/API/APINavigation.h:87 > + void settreatAsSameOriginNavigation(bool value) { m_treatAsSameOriginNavigation = value; } s/sett/setT/
Comment on attachment 338766 [details] Made data: and about: use same process View in context: https://bugs.webkit.org/attachment.cgi?id=338766&action=review >> Source/WebKit/UIProcess/WebProcessPool.cpp:2027 >> + if (navigation.treatAsSameOriginNavigation()) > > Can we simplify the logic here with the lines below checking for !url.isValid() || url.isEmpty() || url.isBlankURL() || protocolHostAndPortAreEqual(url, targetURL)? Maybe WebProcess could do the computation here. > I am wondering what we should do with file based origins/loads as well. Okay, I talked with Brady. We can't move protocolHostAndPortAreEqual because of redirects but we can move isEmpty() and isBlankURL() so let's do that.
Committed r231019: <https://trac.webkit.org/changeset/231019>
(In reply to Ryosuke Niwa from comment #9) > Comment on attachment 338766 [details] > Made data: and about: use same process > > View in context: > https://bugs.webkit.org/attachment.cgi?id=338766&action=review > > >> Source/WebKit/UIProcess/WebProcessPool.cpp:2027 > >> + if (navigation.treatAsSameOriginNavigation()) > > > > Can we simplify the logic here with the lines below checking for !url.isValid() || url.isEmpty() || url.isBlankURL() || protocolHostAndPortAreEqual(url, targetURL)? Maybe WebProcess could do the computation here. > > I am wondering what we should do with file based origins/loads as well. > > Okay, I talked with Brady. We can't move protocolHostAndPortAreEqual because > of redirects but we can move isEmpty() and isBlankURL() so let's do that. Actually, this breaks layout tests. We need to keep isBlankURL() and isEmpty() check here.
It's because the check in WebProcessPool::processForNavigation is about the originating / source URL, not the target URL.
Committed r231031: <https://trac.webkit.org/changeset/231031>