Changeset 148034 in webkit


Ignore:
Timestamp:
Apr 9, 2013 12:04:33 PM (11 years ago)
Author:
timothy_horton@apple.com
Message:

[wk2] IconDatabase images should be decoded in the WebProcess
​https://bugs.webkit.org/show_bug.cgi?id=112524
<rdar://problem/10133914>

Reviewed by Oliver Hunt.

In the interests of keeping decoding of data coming in from the greater Internet
in the WebProcess, move decoding of favicons from the UIProcess to the WebProcess.

  • UIProcess/WebIconDatabase.cpp:

(WebKit::WebIconDatabase::setIconBitmapForIconURL):
Receive a ShareableBitmap handle from the WebProcess instead of a DataReference.
Use the new setIconBitmapForIconURL IconDatabase method.

  • UIProcess/WebIconDatabase.h:

(WebIconDatabase):
Rename setIconDataForIconURL to setIconBitmapForIconURL.

  • UIProcess/WebIconDatabase.messages.in: Ditto.
  • WebProcess/IconDatabase/WebIconDatabaseProxy.cpp:

(WebKit::WebIconDatabaseProxy::setIconDataForIconURL):
In the WebProcess, decode the incoming icon and draw it into a ShareableBitmap.

No testable behavior change.

  • loader/icon/IconDatabase.cpp:

(WebCore::IconDatabase::updateIconRecord):
Added updateIconRecord, which factors most of setIconDataForIconURL out so it can
be shared with setIconBitmapForIconURL. This function will set either a bitmap or
raw image data for the given icon URL.

(WebCore::IconDatabase::setIconBitmapForIconURL):
Added; make a copy of the bitmap for thread-safety purposes, and call updateIconRecord.

(WebCore::IconDatabase::setIconDataForIconURL):
Factored out of what is now updateIconRecord.

  • loader/icon/IconDatabase.h:

(IconDatabase): Add setIconBitmapForIconURL and updateIconRecord.

  • loader/icon/IconDatabaseBase.h:

(WebCore::IconDatabaseBase::setIconBitmapForIconURL): Added.

  • loader/icon/IconRecord.cpp:

(WebCore::IconRecord::setImage): Set the image for an icon record directly (as opposed
to setting the image data, which causes the image to be decoded in the WebProcess).

  • loader/icon/IconRecord.h:

(IconRecord): Add setImage.

Location:
trunk/Source
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r148033 r148034  
     12013-04-09  Tim Horton  <timothy_horton@apple.com>
     2
     3        [wk2] IconDatabase images should be decoded in the WebProcess
     4        https://bugs.webkit.org/show_bug.cgi?id=112524
     5        <rdar://problem/10133914>
     6
     7        Reviewed by Oliver Hunt.
     8
     9        No testable behavior change.
     10
     11        * loader/icon/IconDatabase.cpp:
     12        (WebCore::IconDatabase::updateIconRecord):
     13        Added updateIconRecord, which factors most of setIconDataForIconURL out so it can
     14        be shared with setIconBitmapForIconURL. This function will set either a bitmap or
     15        raw image data for the given icon URL.
     16
     17        (WebCore::IconDatabase::setIconBitmapForIconURL):
     18        Added; make a copy of the bitmap for thread-safety purposes, and call updateIconRecord.
     19
     20        (WebCore::IconDatabase::setIconDataForIconURL):
     21        Factored out of what is now updateIconRecord.
     22
     23        * loader/icon/IconDatabase.h:
     24        (IconDatabase): Add setIconBitmapForIconURL and updateIconRecord.
     25
     26        * loader/icon/IconDatabaseBase.h:
     27        (WebCore::IconDatabaseBase::setIconBitmapForIconURL): Added.
     28        * loader/icon/IconRecord.cpp:
     29        (WebCore::IconRecord::setImage): Set the image for an icon record directly (as opposed
     30        to setting the image data, which causes the image to be decoded in the WebProcess).
     31        * loader/icon/IconRecord.h:
     32        (IconRecord): Add setImage.
     33
    1342013-04-09  Chris Fleizach  <cfleizach@apple.com>
    235

  • trunk/Source/WebCore/loader/icon/IconDatabase.cpp

    r147975 r148034  
    3030#if ENABLE(ICONDATABASE)
    3131
     32#include "BitmapImage.h"
    3233#include "DocumentLoader.h"
    3334#include "FileSystem.h"
    … …  
    3536#include "IconRecord.h"
    3637#include "Image.h"
     38#include "ImageBuffer.h"
    3739#include "IntSize.h"
    3840#include "Logging.h"
    … …  
    534536}
    535537
    536 void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal, const String& iconURLOriginal)
    537 {   
    538     ASSERT_NOT_SYNC_THREAD();
    539    
    540     // Cannot do anything with dataOriginal or iconURLOriginal that would end up storing them without deep copying first
    541    
    542     if (!isOpen() || iconURLOriginal.isEmpty())
    543         return;
    544    
    545     RefPtr<SharedBuffer> data = dataOriginal ? dataOriginal->copy() : PassRefPtr<SharedBuffer>(0);
    546     if (data)
    547         data->setMutexForVerifier(m_urlAndIconLock);
    548     String iconURL = iconURLOriginal.isolatedCopy();
    549    
     538void IconDatabase::updateIconRecord(PassRefPtr<SharedBuffer> iconData, PassRefPtr<Image> iconBitmap, const String& iconURL)
     539{
     540    // Only one of iconData or iconBitmap should be provided, never both.
     541    ASSERT(!iconData != !iconBitmap);
     542
    550543    Vector<String> pageURLs;
    551544    {
    552545        MutexLocker locker(m_urlAndIconLock);
    553    
     546
    554547        // If this icon was pending a read, remove it from that set because this new data should override what is on disk
    555548        RefPtr<IconRecord> icon = m_iconURLToRecordMap.get(iconURL);
    … …  
    559552        } else
    560553            icon = getOrCreateIconRecord(iconURL);
    561    
    562         // Update the data and set the time stamp
    563         icon->setImageData(data.release());
     554
     555        // Update the image and set the time stamp
     556        if (iconData)
     557            icon->setImageData(iconData);
     558        else if (iconBitmap)
     559            icon->setImage(iconBitmap);
    564560        icon->setTimestamp((int)currentTime());
    565        
     561
    566562        // Copy the current retaining pageURLs - if any - to notify them of the change
    567563        pageURLs.appendRange(icon->retainingPageURLs().begin(), icon->retainingPageURLs().end());
    568        
     564
    569565        // Mark the IconRecord as requiring an update to the database only if private browsing is disabled
    570566        if (!m_privateBrowsingEnabled) {
    … …  
    588584
    589585        // Informal testing shows that draining the autorelease pool every 25 iterations is about as low as we can go
    590         // before performance starts to drop off, but we don't want to increase this number because then accumulated memory usage will go up       
     586        // before performance starts to drop off, but we don't want to increase this number because then accumulated memory usage will go up
    591587        AutodrainedPool pool(25);
    592588
    … …  
    598594        }
    599595    }
     596}
     597
     598void IconDatabase::setIconBitmapForIconURL(PassRefPtr<Image> imageOriginal, const String& iconURLOriginal)
     599{
     600    ASSERT_NOT_SYNC_THREAD();
     601    ASSERT(imageOriginal->isBitmapImage());
     602   
     603    // Cannot do anything with imageOriginal or iconURLOriginal that would end up storing them without deep copying first
     604
     605    if (!isOpen() || iconURLOriginal.isEmpty())
     606        return;
     607   
     608    RefPtr<Image> image;
     609    if (imageOriginal) {
     610        OwnPtr<ImageBuffer> imageBuffer = ImageBuffer::create(imageOriginal->size());
     611        GraphicsContext* context = imageBuffer->context();
     612       
     613        context->drawImage(imageOriginal.get(), ColorSpaceDeviceRGB, IntPoint());
     614        image = imageBuffer->copyImage();
     615    }
     616
     617    if (image)
     618        image->setMutexForVerifier(m_urlAndIconLock);
     619
     620    String iconURL = iconURLOriginal.isolatedCopy();
     621
     622    updateIconRecord(0, image.release(), iconURL);
     623}
     624
     625void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal, const String& iconURLOriginal)
     626{   
     627    ASSERT_NOT_SYNC_THREAD();
     628
     629    // Cannot do anything with dataOriginal or iconURLOriginal that would end up storing them without deep copying first
     630   
     631    if (!isOpen() || iconURLOriginal.isEmpty())
     632        return;
     633   
     634    RefPtr<SharedBuffer> data = dataOriginal ? dataOriginal->copy() : PassRefPtr<SharedBuffer>(0);
     635    if (data)
     636        data->setMutexForVerifier(m_urlAndIconLock);
     637
     638    String iconURL = iconURLOriginal.isolatedCopy();
     639
     640    updateIconRecord(data.release(), 0, iconURL);
    600641}
    601642

  • trunk/Source/WebCore/loader/icon/IconDatabase.h

    r147622 r148034  
    9494    virtual void retainIconForPageURL(const String&);
    9595    virtual void releaseIconForPageURL(const String&);
    96     virtual void setIconDataForIconURL(PassRefPtr<SharedBuffer> data, const String&);
     96    virtual void setIconDataForIconURL(PassRefPtr<SharedBuffer> data, const String&) OVERRIDE;
     97    virtual void setIconBitmapForIconURL(PassRefPtr<Image>, const String&) OVERRIDE;
    9798    virtual void setIconURLForPageURL(const String& iconURL, const String& pageURL);
    9899
    … …  
    129130    void scheduleOrDeferSyncTimer();
    130131    void syncTimerFired(Timer<IconDatabase>*);
     132
     133    void updateIconRecord(PassRefPtr<SharedBuffer>, PassRefPtr<Image>, const String&);
    131134   
    132135    Timer<IconDatabase> m_syncTimer;

  • trunk/Source/WebCore/loader/icon/IconDatabaseBase.h

    r147622 r148034  
    2727#define IconDatabaseBase_h
    2828
     29#include "Image.h"
    2930#include "ImageSource.h"
    3031#include "SharedBuffer.h"
    … …  
    173174    virtual void setIconURLForPageURL(const String&, const String&) { }
    174175    virtual void setIconDataForIconURL(PassRefPtr<SharedBuffer>, const String&) { }
     176    virtual void setIconBitmapForIconURL(PassRefPtr<Image>, const String&) { }
    175177
    176178    // Synchronous calls used internally by WebCore.

  • trunk/Source/WebCore/loader/icon/IconRecord.cpp

    r65344 r148034  
    7878}
    7979
     80void IconRecord::setImage(PassRefPtr<Image> image)
     81{
     82    m_image = image;
     83    m_dataSet = true;
     84}
     85
    8086void IconRecord::loadImageFromResource(const char* resource)
    8187{

  • trunk/Source/WebCore/loader/icon/IconRecord.h

    r127757 r148034  
    8686       
    8787    void setImageData(PassRefPtr<SharedBuffer> data);
     88    void setImage(PassRefPtr<Image> data);
    8889    Image* image(const IntSize&);   
    8990   

  • trunk/Source/WebKit2/ChangeLog

    r148025 r148034  
     12013-04-09  Tim Horton  <timothy_horton@apple.com>
     2
     3        [wk2] IconDatabase images should be decoded in the WebProcess
     4        https://bugs.webkit.org/show_bug.cgi?id=112524
     5        <rdar://problem/10133914>
     6
     7        Reviewed by Oliver Hunt.
     8
     9        In the interests of keeping decoding of data coming in from the greater Internet
     10        in the WebProcess, move decoding of favicons from the UIProcess to the WebProcess.
     11
     12        * UIProcess/WebIconDatabase.cpp:
     13        (WebKit::WebIconDatabase::setIconBitmapForIconURL):
     14        Receive a ShareableBitmap handle from the WebProcess instead of a DataReference.
     15        Use the new setIconBitmapForIconURL IconDatabase method.
     16
     17        * UIProcess/WebIconDatabase.h:
     18        (WebIconDatabase):
     19        Rename setIconDataForIconURL to setIconBitmapForIconURL.
     20
     21        * UIProcess/WebIconDatabase.messages.in: Ditto.
     22
     23        * WebProcess/IconDatabase/WebIconDatabaseProxy.cpp:
     24        (WebKit::WebIconDatabaseProxy::setIconDataForIconURL):
     25        In the WebProcess, decode the incoming icon and draw it into a ShareableBitmap.
     26
    1272013-04-09  Raphael Kubo da Costa  <raphael.kubo.da.costa@intel.com>
    228

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

    r140607 r148034  
    122122}
    123123
    124 void WebIconDatabase::setIconDataForIconURL(const CoreIPC::DataReference& iconData, const String& iconURL)
    125 {
    126     LOG(IconDatabase, "WK2 UIProcess setting icon data (%i bytes) for page URL %s", (int)iconData.size(), iconURL.ascii().data());
     124void WebIconDatabase::setIconBitmapForIconURL(const ShareableBitmap::Handle& bitmapHandle, const String& iconURL)
     125{
     126    LOG(IconDatabase, "WK2 UIProcess setting icon bitmap for page URL %s", iconURL.ascii().data());
    127127    if (!m_iconDatabaseImpl)
    128128        return;
    129129
    130     RefPtr<SharedBuffer> buffer = SharedBuffer::create(iconData.data(), iconData.size());
    131     m_iconDatabaseImpl->setIconDataForIconURL(buffer.release(), iconURL);
     130    if (bitmapHandle.isNull()) {
     131        m_iconDatabaseImpl->setIconBitmapForIconURL(0, iconURL);
     132        return;
     133    }
     134
     135    RefPtr<ShareableBitmap> iconBitmap = ShareableBitmap::create(bitmapHandle, SharedMemory::ReadOnly);
     136    m_iconDatabaseImpl->setIconBitmapForIconURL(iconBitmap->createImage(), iconURL);
    132137}
    133138

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

    r147403 r148034  
    3030
    3131#include "Connection.h"
     32#include "ShareableBitmap.h"
    3233#include "WebIconDatabaseClient.h"
    3334#include <WebCore/IconDatabaseClient.h>
    … …  
    6768    void releaseIconForPageURL(const String&);
    6869    void setIconURLForPageURL(const String&, const String&);
    69     void setIconDataForIconURL(const CoreIPC::DataReference&, const String&);
     70    void setIconBitmapForIconURL(const ShareableBitmap::Handle&, const String&);
    7071   
    7172    void synchronousIconDataForPageURL(const String&, CoreIPC::DataReference&);

  • trunk/Source/WebKit2/UIProcess/WebIconDatabase.messages.in

    r140607 r148034  
    2525    ReleaseIconForPageURL(WTF::String pageURL)
    2626    SetIconURLForPageURL(WTF::String iconURL, WTF::String pageURL)
    27     SetIconDataForIconURL(CoreIPC::DataReference iconData, WTF::String iconURL)
     27    SetIconBitmapForIconURL(WebKit::ShareableBitmap::Handle bitmapHandle, WTF::String iconURL)
    2828   
    2929    SynchronousIconDataForPageURL(WTF::String pageURL) -> (CoreIPC::DataReference iconData)

  • trunk/Source/WebKit2/WebProcess/IconDatabase/WebIconDatabaseProxy.cpp

    r140607 r148034  
    3131#include "WebIconDatabaseProxyMessages.h"
    3232#include "WebProcess.h"
     33#include <WebCore/BitmapImage.h>
     34#include <WebCore/GraphicsContext.h>
     35#include <WebCore/IntPoint.h>
    3336#include <WebCore/SharedBuffer.h>
    3437#include <wtf/text/WTFString.h>
    … …  
    136139void WebIconDatabaseProxy::setIconDataForIconURL(PassRefPtr<SharedBuffer> iconData, const String& iconURL)
    137140{
    138     CoreIPC::DataReference data(reinterpret_cast<const uint8_t*>(iconData ? iconData->data() : 0), iconData ? iconData->size() : 0);
    139     m_process->connection()->send(Messages::WebIconDatabase::SetIconDataForIconURL(data, iconURL), 0);
     141    RefPtr<Image> image = BitmapImage::create();
     142    ShareableBitmap::Handle handle;
     143
     144    if (image->setData(iconData, true) && !image->size().isEmpty()) {
     145        RefPtr<ShareableBitmap> shareableBitmap = ShareableBitmap::createShareable(image->size(), ShareableBitmap::SupportsAlpha);
     146        OwnPtr<GraphicsContext> bitmapContext = shareableBitmap->createGraphicsContext();
     147        bitmapContext->drawImage(image.get(), ColorSpaceDeviceRGB, IntPoint());
     148
     149        shareableBitmap->createHandle(handle, SharedMemory::ReadOnly);
     150    }
     151
     152    m_process->connection()->send(Messages::WebIconDatabase::SetIconBitmapForIconURL(handle, iconURL), 0);
    140153}
    141154

  • trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm

    r145477 r148034  
    346346    m_pdfLayerController.get().delegate = 0;
    347347
    348     if (webFrame()) {
     348    if (webFrame() && webFrame()->coreFrame()) {
    349349        if (FrameView* frameView = webFrame()->coreFrame()->view())
    350350            frameView->removeScrollableArea(this);
Note: See TracChangeset for help on using the changeset viewer.