Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Unreviewed, rolling out r212786.
https://bugs.webkit.org/show_bug.cgi?id=168710

It broke GTK+ port when using single shared process model
(Requested by KaL on #webkit).

Reverted changeset:

"Refactor WebViewImpl creation in preparation for supporting
multiple WebsiteDataStores."
https://bugs.webkit.org/show_bug.cgi?id=168676
http://trac.webkit.org/changeset/212786

Canonical link: https://commits.webkit.org/185715@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@212810 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
webkit-commit-queue committed Feb 22, 2017
1 parent 3517b07 commit 5bef28b
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 53 deletions.
15 changes: 15 additions & 0 deletions Source/WebKit2/ChangeLog
@@ -1,3 +1,18 @@
2017-02-21 Commit Queue <commit-queue@webkit.org>

Unreviewed, rolling out r212786.
https://bugs.webkit.org/show_bug.cgi?id=168710

It broke GTK+ port when using single shared process model
(Requested by KaL on #webkit).

Reverted changeset:

"Refactor WebViewImpl creation in preparation for supporting
multiple WebsiteDataStores."
https://bugs.webkit.org/show_bug.cgi?id=168676
http://trac.webkit.org/changeset/212786

2017-02-21 Brady Eidson <beidson@apple.com>

Refactor WebViewImpl creation in preparation for supporting multiple WebsiteDataStores.
Expand Down
7 changes: 1 addition & 6 deletions Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Expand Up @@ -549,12 +549,7 @@ - (void)_initializeWithConfiguration:(WKWebViewConfiguration *)configuration
#endif

#if PLATFORM(MAC)
_impl = WebKit::WebViewImpl::maybeCreate(self, self, processPool, WTFMove(pageConfiguration));
if (!_impl) {
[NSException raise:NSInternalInconsistencyException format:@"WKWebView initialization failed. Unable to create a new WebProcess."];
return;
}

_impl = std::make_unique<WebKit::WebViewImpl>(self, self, processPool, WTFMove(pageConfiguration));
_page = &_impl->page();

_impl->setAutomaticallyAdjustsContentInsets(true);
Expand Down
7 changes: 1 addition & 6 deletions Source/WebKit2/UIProcess/API/mac/WKView.mm
Expand Up @@ -904,12 +904,7 @@ - (instancetype)initWithFrame:(NSRect)frame processPool:(WebProcessPool&)process
InitializeWebKit2();

_data = [[WKViewData alloc] init];

_data->_impl = WebViewImpl::maybeCreate(self, nullptr, processPool, WTFMove(configuration));
if (!_data->_impl) {
[NSException raise:NSInternalInconsistencyException format:@"[WKView initWithFrame:processPool:configuration:] failed to create a new WebProcess"];
return nil;
}
_data->_impl = std::make_unique<WebViewImpl>(self, nullptr, processPool, WTFMove(configuration));

[self maybeInstallIconLoadingClient];

Expand Down
4 changes: 1 addition & 3 deletions Source/WebKit2/UIProcess/Cocoa/WebViewImpl.h
Expand Up @@ -120,7 +120,7 @@ class WebViewImpl {
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(WebViewImpl);
public:
static std::unique_ptr<WebViewImpl> maybeCreate(NSView <WebViewImplDelegate> *, WKWebView *outerWebView, WebProcessPool&, Ref<API::PageConfiguration>&&);
WebViewImpl(NSView <WebViewImplDelegate> *, WKWebView *outerWebView, WebProcessPool&, Ref<API::PageConfiguration>&&);

~WebViewImpl();

Expand Down Expand Up @@ -523,8 +523,6 @@ class WebViewImpl {
#endif // HAVE(TOUCH_BAR)

private:
WebViewImpl(NSView <WebViewImplDelegate> *, std::unique_ptr<PageClientImpl>&&, Ref<WebPageProxy>&&, WebProcessPool&);

#if HAVE(TOUCH_BAR)
void setUpTextTouchBar(NSTouchBar *);
void updateTextTouchBar();
Expand Down
16 changes: 3 additions & 13 deletions Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm
Expand Up @@ -1229,20 +1229,10 @@ static NSTrackingAreaOptions trackingAreaOptions()
return options;
}

std::unique_ptr<WebViewImpl> WebViewImpl::maybeCreate(NSView <WebViewImplDelegate> *view, WKWebView *outerWebView, WebProcessPool& processPool, Ref<API::PageConfiguration>&& configuration)
{
auto pageClient = std::make_unique<PageClientImpl>(view, outerWebView);
auto webPage = processPool.createWebPage(*pageClient, WTFMove(configuration));
if (!webPage)
return nullptr;

return std::unique_ptr<WebViewImpl>(new WebViewImpl(view, WTFMove(pageClient), *webPage, processPool));
}

WebViewImpl::WebViewImpl(NSView <WebViewImplDelegate> *view, std::unique_ptr<PageClientImpl>&& pageClient, Ref<WebPageProxy>&& page, WebProcessPool& processPool)
WebViewImpl::WebViewImpl(NSView <WebViewImplDelegate> *view, WKWebView *outerWebView, WebProcessPool& processPool, Ref<API::PageConfiguration>&& configuration)
: m_view(view)
, m_pageClient(WTFMove(pageClient))
, m_page(WTFMove(page))
, m_pageClient(std::make_unique<PageClientImpl>(view, outerWebView))
, m_page(processPool.createWebPage(*m_pageClient, WTFMove(configuration)))
, m_weakPtrFactory(this)
, m_needsViewFrameInWindowCoordinates(m_page->preferences().pluginsEnabled())
, m_intrinsicContentSize(CGSizeMake(NSViewNoInstrinsicMetric, NSViewNoInstrinsicMetric))
Expand Down
7 changes: 1 addition & 6 deletions Source/WebKit2/UIProcess/WebPageProxy.cpp
Expand Up @@ -716,13 +716,8 @@ void WebPageProxy::reattachToWebProcess()
m_process->removeWebPage(m_pageID);
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);

auto* newProcess = m_process->processPool().maybeCreateNewWebProcessRespectingProcessCountLimit(m_websiteDataStore.ptr());
if (!newProcess) {
WTFLogAlways("Couldn't reattach to a new web process");
return;
}
m_process = m_process->processPool().createNewWebProcessRespectingProcessCountLimit();

m_process = *newProcess;
ASSERT(m_process->state() != ChildProcessProxy::State::Terminated);
if (m_process->state() == ChildProcessProxy::State::Running)
processDidFinishLaunching();
Expand Down
25 changes: 9 additions & 16 deletions Source/WebKit2/UIProcess/WebProcessPool.cpp
Expand Up @@ -557,7 +557,7 @@ void WebProcessPool::resolvePathsForSandboxExtensions()
platformResolvePathsForSandboxExtensions();
}

WebProcessProxy& WebProcessPool::createNewWebProcess(WebsiteDataStore* websiteDataStore)
WebProcessProxy& WebProcessPool::createNewWebProcess()
{
ensureNetworkProcess();

Expand Down Expand Up @@ -693,7 +693,7 @@ void WebProcessPool::warmInitialProcess()
if (m_processes.size() >= maximumNumberOfProcesses())
return;

createNewWebProcess(nullptr);
createNewWebProcess();
m_haveInitialEmptyProcess = true;
}

Expand Down Expand Up @@ -769,25 +769,20 @@ void WebProcessPool::disconnectProcess(WebProcessProxy* process)
#endif
}

WebProcessProxy* WebProcessPool::maybeCreateNewWebProcessRespectingProcessCountLimit(WebsiteDataStore* websiteDataStore)
WebProcessProxy& WebProcessPool::createNewWebProcessRespectingProcessCountLimit()
{
if (m_processes.size() < maximumNumberOfProcesses())
return &createNewWebProcess(websiteDataStore);

// If a non-default WebsiteDataStore was passed in and we couldn't make a new web process for it,
// we should not return an existing process.
if (websiteDataStore && websiteDataStore != &API::WebsiteDataStore::defaultDataStore()->websiteDataStore())
return nullptr;
return createNewWebProcess();

// Choose the process with fewest pages.
auto& process = *std::min_element(m_processes.begin(), m_processes.end(), [](const RefPtr<WebProcessProxy>& a, const RefPtr<WebProcessProxy>& b) {
return a->pageCount() < b->pageCount();
});

return process.get();
return *process;
}

RefPtr<WebPageProxy> WebProcessPool::createWebPage(PageClient& pageClient, Ref<API::PageConfiguration>&& pageConfiguration)
Ref<WebPageProxy> WebProcessPool::createWebPage(PageClient& pageClient, Ref<API::PageConfiguration>&& pageConfiguration)
{
if (!pageConfiguration->pageGroup())
pageConfiguration->setPageGroup(m_defaultPageGroup.ptr());
Expand All @@ -797,23 +792,21 @@ RefPtr<WebPageProxy> WebProcessPool::createWebPage(PageClient& pageClient, Ref<A
pageConfiguration->setUserContentController(&pageConfiguration->pageGroup()->userContentController());
if (!pageConfiguration->visitedLinkStore())
pageConfiguration->setVisitedLinkStore(m_visitedLinkStore.ptr());

bool pageHasWebsiteDataStore = pageConfiguration->websiteDataStore();
if (!pageHasWebsiteDataStore) {
if (!pageConfiguration->websiteDataStore()) {
ASSERT(!pageConfiguration->sessionID().isValid());
pageConfiguration->setWebsiteDataStore(m_websiteDataStore.get());
pageConfiguration->setSessionID(pageConfiguration->preferences()->privateBrowsingEnabled() ? SessionID::legacyPrivateSessionID() : SessionID::defaultSessionID());
}

RefPtr<WebProcessProxy> process;
if (m_haveInitialEmptyProcess && !pageHasWebsiteDataStore) {
if (m_haveInitialEmptyProcess) {
process = m_processes.last();
m_haveInitialEmptyProcess = false;
} else if (pageConfiguration->relatedPage()) {
// Sharing processes, e.g. when creating the page via window.open().
process = &pageConfiguration->relatedPage()->process();
} else
process = maybeCreateNewWebProcessRespectingProcessCountLimit(&pageConfiguration->websiteDataStore()->websiteDataStore());
process = &createNewWebProcessRespectingProcessCountLimit();

return process->createWebPage(pageClient, WTFMove(pageConfiguration));
}
Expand Down
6 changes: 3 additions & 3 deletions Source/WebKit2/UIProcess/WebProcessPool.h
Expand Up @@ -170,7 +170,7 @@ class WebProcessPool final : public API::ObjectImpl<API::Object::Type::ProcessPo

API::WebsiteDataStore* websiteDataStore() const { return m_websiteDataStore.get(); }

RefPtr<WebPageProxy> createWebPage(PageClient&, Ref<API::PageConfiguration>&&);
Ref<WebPageProxy> createWebPage(PageClient&, Ref<API::PageConfiguration>&&);

const String& injectedBundlePath() const { return m_configuration->injectedBundlePath(); }

Expand Down Expand Up @@ -258,7 +258,7 @@ class WebProcessPool final : public API::ObjectImpl<API::Object::Type::ProcessPo

void allowSpecificHTTPSCertificateForHost(const WebCertificateInfo*, const String& host);

WebProcessProxy* maybeCreateNewWebProcessRespectingProcessCountLimit(WebsiteDataStore*); // Will return an existing one if limit is met.
WebProcessProxy& createNewWebProcessRespectingProcessCountLimit(); // Will return an existing one if limit is met.
void warmInitialProcess();

bool shouldTerminate(WebProcessProxy*);
Expand Down Expand Up @@ -397,7 +397,7 @@ class WebProcessPool final : public API::ObjectImpl<API::Object::Type::ProcessPo
void platformInitializeWebProcess(WebProcessCreationParameters&);
void platformInvalidateContext();

WebProcessProxy& createNewWebProcess(WebsiteDataStore*);
WebProcessProxy& createNewWebProcess();

void requestWebContentStatistics(StatisticsRequest*);
void requestNetworkingStatistics(StatisticsRequest*);
Expand Down

0 comments on commit 5bef28b

Please sign in to comment.