Changeset 212810 in webkit

Timestamp:

Feb 21, 2017, 11:43:33 PM (10 years ago)

Author:

commit-queue@webkit.org

Message:

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

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/Cocoa/WKWebView.mm (modified) (1 diff)
• UIProcess/API/mac/WKView.mm (modified) (1 diff)
• UIProcess/Cocoa/WebViewImpl.h (modified) (2 diffs)
• UIProcess/Cocoa/WebViewImpl.mm (modified) (1 diff)
• UIProcess/WebPageProxy.cpp (modified) (1 diff)
• UIProcess/WebProcessPool.cpp (modified) (7 diffs)
• UIProcess/WebProcessPool.h (modified) (3 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm

@@ -550 +550 @@
 
 #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();
 

trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm

@@ -905 +905 @@
 
     _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];

trunk/Source/WebKit2/UIProcess/Cocoa/WebViewImpl.h

@@ -121 +121 @@
     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();
@@ -524 +524 @@
 
 private:
-    WebViewImpl(NSView <WebViewImplDelegate> *, std::unique_ptr<PageClientImpl>&&, Ref<WebPageProxy>&&, WebProcessPool&);
-
 #if HAVE(TOUCH_BAR)
     void setUpTextTouchBar(NSTouchBar *);

trunk/Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm

@@ -1230 +1230 @@
 }
 
-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())

trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp

@@ -717 +717 @@
     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 = *newProcess;
+    m_process = m_process->processPool().createNewWebProcessRespectingProcessCountLimit();
+
     ASSERT(m_process->state() != ChildProcessProxy::State::Terminated);
     if (m_process->state() == ChildProcessProxy::State::Running)

trunk/Source/WebKit2/UIProcess/WebProcessPool.cpp

@@ -558 +558 @@
 }
 
-WebProcessProxy& WebProcessPool::createNewWebProcess(WebsiteDataStore* websiteDataStore)
+WebProcessProxy& WebProcessPool::createNewWebProcess()
 {
     ensureNetworkProcess();
@@ -694 +694 @@
         return;
 
-    createNewWebProcess(nullptr);
+    createNewWebProcess();
     m_haveInitialEmptyProcess = true;
 }
@@ -770 +770 @@
 }
 
-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.
@@ -785 +780 @@
     });
 
-    return process.get();
-}
-
-RefPtr<WebPageProxy> WebProcessPool::createWebPage(PageClient& pageClient, Ref<API::PageConfiguration>&& pageConfiguration)
+    return *process;
+}
+
+Ref<WebPageProxy> WebProcessPool::createWebPage(PageClient& pageClient, Ref<API::PageConfiguration>&& pageConfiguration)
 {
     if (!pageConfiguration->pageGroup())
@@ -798 +793 @@
     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());
@@ -807 +800 @@
 
     RefPtr<WebProcessProxy> process;
-    if (m_haveInitialEmptyProcess && !pageHasWebsiteDataStore) {
+    if (m_haveInitialEmptyProcess) {
         process = m_processes.last();
         m_haveInitialEmptyProcess = false;
@@ -814 +807 @@
         process = &pageConfiguration->relatedPage()->process();
     } else
-        process = maybeCreateNewWebProcessRespectingProcessCountLimit(&pageConfiguration->websiteDataStore()->websiteDataStore());
+        process = &createNewWebProcessRespectingProcessCountLimit();
 
     return process->createWebPage(pageClient, WTFMove(pageConfiguration));

trunk/Source/WebKit2/UIProcess/WebProcessPool.h

@@ -171 +171 @@
     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(); }
@@ -259 +259 @@
     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();
 
@@ -398 +398 @@
     void platformInvalidateContext();
 
-    WebProcessProxy& createNewWebProcess(WebsiteDataStore*);
+    WebProcessProxy& createNewWebProcess();
 
     void requestWebContentStatistics(StatisticsRequest*);