Changeset 172660 in webkit


Ignore:
Timestamp:
Aug 15, 2014, 4:56:57 PM (11 years ago)
Author:
ap@apple.com
Message:

Improve page to process relationship tracking
​https://bugs.webkit.org/show_bug.cgi?id=135996
<rdar://problem/16991213>

Reviewed by Sam Weinig.

  • UIProcess/VisitedLinkProvider.cpp:

(WebKit::VisitedLinkProvider::removeAll):
(WebKit::VisitedLinkProvider::pendingVisitedLinksTimerFired):
(WebKit::VisitedLinkProvider::sendTable):
Added assertions for m_processes only having valid entries.

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::reattachToWebProcess): When attaching to a new process,
tell the old process that the page is not longer associated with it, avoiding
a potential stale pointer.
If re-attached to an existing process, make sure that we perform all the same
registrations as after having launched a new process. This substantially improves
the behavior when the number of open tabs is over process limit.
(WebKit::WebPageProxy::reattachToWebProcessWithItem): Added ASSERT(!isValid())
to avoid confusion. All other calls to reattachToWebProcess() have this as a
runtime check, but reattachToWebProcessWithItem() is only called for valid pages.
(WebKit::WebPageProxy::terminateProcess): Added an assertion with a FIXME for
something that will need to be fixed another day.

  • UIProcess/WebPageProxy.h: Removed an unimplemented function.
  • UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::addExistingWebPage): Added assertions for page map sanity.
(WebKit::WebProcessProxy::removeWebPage): Added a check for page state being Terminated
already. This avoids an assertion failure that happened under the new call to
removeWebPage() in reattachToWebProcess(), as we are now calling it for terminated
processes that are not in WebContext::m_processes any more.
(WebKit::WebProcessProxy::didFinishLaunching): Added an assertion that page agrees
about using this process.

Location:
trunk/Source/WebKit2
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit2/ChangeLog

    r172653 r172660  
     12014-08-15  Alexey Proskuryakov  <ap@apple.com>
     2
     3        Improve page to process relationship tracking
     4        https://bugs.webkit.org/show_bug.cgi?id=135996
     5        <rdar://problem/16991213>
     6
     7        Reviewed by Sam Weinig.
     8
     9        * UIProcess/VisitedLinkProvider.cpp:
     10        (WebKit::VisitedLinkProvider::removeAll):
     11        (WebKit::VisitedLinkProvider::pendingVisitedLinksTimerFired):
     12        (WebKit::VisitedLinkProvider::sendTable):
     13        Added assertions for m_processes only having valid entries.
     14
     15        * UIProcess/WebPageProxy.cpp:
     16        (WebKit::WebPageProxy::reattachToWebProcess): When attaching to a new process,
     17        tell the old process that the page is not longer associated with it, avoiding
     18        a potential stale pointer.
     19        If re-attached to an existing process, make sure that we perform all the same
     20        registrations as after having launched a new process. This substantially improves
     21        the behavior when the number of open tabs is over process limit.
     22        (WebKit::WebPageProxy::reattachToWebProcessWithItem): Added ASSERT(!isValid())
     23        to avoid confusion. All other calls to reattachToWebProcess() have this as a
     24        runtime check, but reattachToWebProcessWithItem() is only called for valid pages.
     25        (WebKit::WebPageProxy::terminateProcess): Added an assertion with a FIXME for
     26        something that will need to be fixed another day.
     27
     28        * UIProcess/WebPageProxy.h: Removed an unimplemented function.
     29
     30        * UIProcess/WebProcessProxy.cpp:
     31        (WebKit::WebProcessProxy::addExistingWebPage): Added assertions for page map sanity.
     32        (WebKit::WebProcessProxy::removeWebPage): Added a check for page state being Terminated
     33        already. This avoids an assertion failure that happened under the new call to
     34        removeWebPage() in reattachToWebProcess(), as we are now calling it for terminated
     35        processes that are not in WebContext::m_processes any more.
     36        (WebKit::WebProcessProxy::didFinishLaunching): Added an assertion that page agrees
     37        about using this process.
     38
    1392014-08-15  Gavin Barraclough  <barraclough@apple.com>
    240

  • trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.cpp

    r168317 r172660  
    9898    m_table.clear();
    9999
    100     for (auto& processAndCount : m_processes)
    101         processAndCount.key->connection()->send(Messages::VisitedLinkTableController::RemoveAllVisitedLinks(), m_identifier);
     100    for (auto& processAndCount : m_processes) {
     101        WebProcessProxy* process = processAndCount.key;
     102        ASSERT(process->context().processes().contains(process));
     103        process->connection()->send(Messages::VisitedLinkTableController::RemoveAllVisitedLinks(), m_identifier);
     104    }
    102105}
    103106
    … …  
    176179
    177180    for (auto& processAndCount : m_processes) {
     181        WebProcessProxy* process = processAndCount.key;
     182        ASSERT(process->context().processes().contains(process));
     183
    178184        if (addedVisitedLinks.size() > 20)
    179             processAndCount.key->connection()->send(Messages::VisitedLinkTableController::AllVisitedLinkStateChanged(), m_identifier);
     185            process->connection()->send(Messages::VisitedLinkTableController::AllVisitedLinkStateChanged(), m_identifier);
    180186        else
    181             processAndCount.key->connection()->send(Messages::VisitedLinkTableController::VisitedLinkStateChanged(addedVisitedLinks), m_identifier);
     187            process->connection()->send(Messages::VisitedLinkTableController::VisitedLinkStateChanged(addedVisitedLinks), m_identifier);
    182188    }
    183189}
    … …  
    230236void VisitedLinkProvider::sendTable(WebProcessProxy& process)
    231237{
     238    ASSERT(process.context().processes().contains(&process));
     239
    232240    SharedMemory::Handle handle;
    233241    if (!m_table.sharedMemory()->createHandle(handle, SharedMemory::ReadOnly))

  • trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp

    r172635 r172660  
    546546
    547547    m_isValid = true;
     548    m_process->removeWebPage(m_pageID);
    548549
    549550    if (m_process->context().processModel() == ProcessModelSharedSecondaryProcess)
    … …  
    551552    else
    552553        m_process = m_process->context().createNewWebProcessRespectingProcessCountLimit();
     554
     555    ASSERT(m_process->state() != ChildProcessProxy::State::Terminated);
     556    if (m_process->state() == ChildProcessProxy::State::Running)
     557        processDidFinishLaunching();
    553558    m_process->addExistingWebPage(this, m_pageID);
    554559    m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID, *this);
    … …  
    580585    if (item && item != m_backForwardList->currentItem())
    581586        m_backForwardList->goToItem(item);
    582    
     587
     588    ASSERT(!isValid());
    583589    reattachToWebProcess();
    584590
    … …  
    18661872void WebPageProxy::terminateProcess()
    18671873{
     1874    // requestTermination() is a no-op for launching processes, so we get into an inconsistent state by calling resetStateAfterProcessExited().
     1875    // FIXME: A client can terminate the page at any time, so we should do something more meaningful than assert and fall apart in release builds.
     1876    ASSERT(m_process->state() != WebProcessProxy::State::Launching);
     1877
    18681878    // NOTE: This uses a check of m_isValid rather than calling isValid() since
    18691879    // we want this to run even for pages being closed or that already closed.

  • trunk/Source/WebKit2/UIProcess/WebPageProxy.h

    r172635 r172660  
    741741    bool isValid() const;
    742742
    743     PassRefPtr<API::Array> relatedPages() const;
    744 
    745743    const String& urlAtProcessExit() const { return m_urlAtProcessExit; }
    746744    FrameLoadState::State loadStateAtProcessExit() const { return m_loadStateAtProcessExit; }

  • trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp

    r170902 r172660  
    187187void WebProcessProxy::addExistingWebPage(WebPageProxy* webPage, uint64_t pageID)
    188188{
     189    ASSERT(!m_pageMap.contains(pageID));
     190    ASSERT(!globalPageMap().contains(pageID));
     191
    189192    m_pageMap.set(pageID, webPage);
    190193    globalPageMap().set(pageID, webPage);
    … …  
    216219    // If this was the last WebPage open in that web process, and we have no other reason to keep it alive, let it go.
    217220    // We only allow this when using a network process, as otherwise the WebProcess needs to preserve its session state.
    218     if (!m_context->usesNetworkProcess() || !canTerminateChildProcess())
     221    if (!m_context->usesNetworkProcess() || state() == State::Terminated || !canTerminateChildProcess())
    219222        return;
    220223
    … …  
    472475    ChildProcessProxy::didFinishLaunching(launcher, connectionIdentifier);
    473476
    474     for (auto& page : m_pageMap.values())
     477    for (WebPageProxy* page : m_pageMap.values()) {
     478        ASSERT(this == &page->process());
    475479        page->processDidFinishLaunching();
     480    }
    476481
    477482    m_webConnection = WebConnectionToWebProcess::create(this);
Note: See TracChangeset for help on using the changeset viewer.