Unreviewed, rolling out r148034, r148052, r148097, and

r148194.
http://trac.webkit.org/changeset/148034
http://trac.webkit.org/changeset/148052
http://trac.webkit.org/changeset/148097
http://trac.webkit.org/changeset/148194
https://bugs.webkit.org/show_bug.cgi?id=114463

broke mutiresolution favicons, among other things (Requested
by thorton on #webkit).


Source/WebCore:

* loader/icon/IconDatabase.cpp:
(WebCore::IconDatabase::setIconDataForIconURL):
* loader/icon/IconDatabase.h:
(IconDatabase):
* loader/icon/IconDatabaseBase.h:
* loader/icon/IconRecord.cpp:
* loader/icon/IconRecord.h:
(IconRecord):

Source/WebKit2:

* UIProcess/WebIconDatabase.cpp:
(WebKit::WebIconDatabase::setIconDataForIconURL):
* UIProcess/WebIconDatabase.h:
(WebIconDatabase):
* UIProcess/WebIconDatabase.messages.in:
* WebProcess/IconDatabase/WebIconDatabaseProxy.cpp:
(WebKit::WebIconDatabaseProxy::setIconDataForIconURL):
* WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::destroy):


Canonical link: https://commits.webkit.org/132846@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@148233 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,25 @@
+2013-04-11  Commit Queue  <rniwa@webkit.org>
+
+        Unreviewed, rolling out r148034, r148052, r148097, and
+        r148194.
+        http://trac.webkit.org/changeset/148034
+        http://trac.webkit.org/changeset/148052
+        http://trac.webkit.org/changeset/148097
+        http://trac.webkit.org/changeset/148194
+        https://bugs.webkit.org/show_bug.cgi?id=114463
+
+        broke mutiresolution favicons, among other things (Requested
+        by thorton on #webkit).
+
+        * loader/icon/IconDatabase.cpp:
+        (WebCore::IconDatabase::setIconDataForIconURL):
+        * loader/icon/IconDatabase.h:
+        (IconDatabase):
+        * loader/icon/IconDatabaseBase.h:
+        * loader/icon/IconRecord.cpp:
+        * loader/icon/IconRecord.h:
+        (IconRecord):
+
 2013-04-11  ChangSeok Oh  <changseok.oh@collabora.com>
 
         [GTK][AC] Implement GraphicsLayerClutter::moveOrCopyAnimations

Source/WebCore/loader/icon/IconDatabase.cpp

@@ -29,13 +29,11 @@
 
 #if ENABLE(ICONDATABASE)
 
-#include "BitmapImage.h"
 #include "DocumentLoader.h"
 #include "FileSystem.h"
 #include "IconDatabaseClient.h"
 #include "IconRecord.h"
 #include "Image.h"
-#include "ImageBuffer.h"
 #include "IntSize.h"
 #include "Logging.h"
 #include "SQLiteStatement.h"
@@ -535,33 +533,39 @@ void IconDatabase::performReleaseIconForPageURL(const String& pageURLOriginal, i
     delete pageRecord;
 }
 
-void IconDatabase::updateIconRecord(PassRefPtr<SharedBuffer> iconData, PassRefPtr<Image> iconBitmap, const String& iconURL)
-{
-    // Only one of iconData or iconBitmap should be provided, never both. None is also fine.
-    ASSERT(!(iconData && iconBitmap));
-
+void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal, const String& iconURLOriginal)
+{    
+    ASSERT_NOT_SYNC_THREAD();
+
+    // Cannot do anything with dataOriginal or iconURLOriginal that would end up storing them without deep copying first
+
+    if (!isOpen() || iconURLOriginal.isEmpty())
+        return;
+
+    RefPtr<SharedBuffer> data = dataOriginal ? dataOriginal->copy() : PassRefPtr<SharedBuffer>(0);
+    if (data)
+        data->setMutexForVerifier(m_urlAndIconLock);
+    String iconURL = iconURLOriginal.isolatedCopy();
+
     Vector<String> pageURLs;
     {
         MutexLocker locker(m_urlAndIconLock);
-
+    
         // If this icon was pending a read, remove it from that set because this new data should override what is on disk
         RefPtr<IconRecord> icon = m_iconURLToRecordMap.get(iconURL);
         if (icon) {
             MutexLocker locker(m_pendingReadingLock);
             m_iconsPendingReading.remove(icon.get());
         } else
             icon = getOrCreateIconRecord(iconURL);
-
-        // Update the image and set the time stamp
-        if (iconData)
-            icon->setImageData(iconData);
-        else if (iconBitmap)
-            icon->setImage(iconBitmap);
+
+        // Update the data and set the time stamp
+        icon->setImageData(data.release());
         icon->setTimestamp((int)currentTime());
-
+        
         // Copy the current retaining pageURLs - if any - to notify them of the change
         pageURLs.appendRange(icon->retainingPageURLs().begin(), icon->retainingPageURLs().end());
-
+        
         // Mark the IconRecord as requiring an update to the database only if private browsing is disabled
         if (!m_privateBrowsingEnabled) {
             MutexLocker locker(m_pendingSyncLock);
@@ -583,7 +587,7 @@ void IconDatabase::updateIconRecord(PassRefPtr<SharedBuffer> iconData, PassRefPt
         scheduleOrDeferSyncTimer();
 
         // Informal testing shows that draining the autorelease pool every 25 iterations is about as low as we can go
-        // before performance starts to drop off, but we don't want to increase this number because then accumulated memory usage will go up
+        // before performance starts to drop off, but we don't want to increase this number because then accumulated memory usage will go up        
         AutodrainedPool pool(25);
 
         for (unsigned i = 0; i < pageURLs.size(); ++i) {
@@ -595,52 +599,6 @@ void IconDatabase::updateIconRecord(PassRefPtr<SharedBuffer> iconData, PassRefPt
     }
 }
 
-void IconDatabase::setIconBitmapForIconURL(PassRefPtr<Image> imageOriginal, const String& iconURLOriginal)
-{
-    ASSERT_NOT_SYNC_THREAD();
-
-    // Cannot do anything with imageOriginal or iconURLOriginal that would end up storing them without deep copying first
-
-    if (!isOpen() || iconURLOriginal.isEmpty())
-        return;
-
-    RefPtr<Image> image;
-    if (imageOriginal) {
-        ASSERT(imageOriginal->isBitmapImage());
-
-        OwnPtr<ImageBuffer> imageBuffer = ImageBuffer::create(imageOriginal->size());
-        GraphicsContext* context = imageBuffer->context();
-
-        context->drawImage(imageOriginal.get(), ColorSpaceDeviceRGB, IntPoint());
-        image = imageBuffer->copyImage();
-    }
-
-    if (image)
-        image->setMutexForVerifier(m_urlAndIconLock);
-
-    String iconURL = iconURLOriginal.isolatedCopy();
-
-    updateIconRecord(0, image.release(), iconURL);
-}
-
-void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal, const String& iconURLOriginal)
-{    
-    ASSERT_NOT_SYNC_THREAD();
-
-    // Cannot do anything with dataOriginal or iconURLOriginal that would end up storing them without deep copying first
-
-    if (!isOpen() || iconURLOriginal.isEmpty())
-        return;
-
-    RefPtr<SharedBuffer> data = dataOriginal ? dataOriginal->copy() : PassRefPtr<SharedBuffer>(0);
-    if (data)
-        data->setMutexForVerifier(m_urlAndIconLock);
-
-    String iconURL = iconURLOriginal.isolatedCopy();
-
-    updateIconRecord(data.release(), 0, iconURL);
-}
-
 void IconDatabase::setIconURLForPageURL(const String& iconURLOriginal, const String& pageURLOriginal)
 {    
     ASSERT_NOT_SYNC_THREAD();

Source/WebCore/loader/icon/IconDatabase.h

@@ -93,8 +93,7 @@ class IconDatabase : public IconDatabaseBase {
 
     virtual void retainIconForPageURL(const String&);
     virtual void releaseIconForPageURL(const String&);
-    virtual void setIconDataForIconURL(PassRefPtr<SharedBuffer> data, const String&) OVERRIDE;
-    virtual void setIconBitmapForIconURL(PassRefPtr<Image>, const String&) OVERRIDE;
+    virtual void setIconDataForIconURL(PassRefPtr<SharedBuffer> data, const String&);
     virtual void setIconURLForPageURL(const String& iconURL, const String& pageURL);
 
     virtual Image* synchronousIconForPageURL(const String&, const IntSize&);
@@ -129,8 +128,6 @@ class IconDatabase : public IconDatabaseBase {
     void wakeSyncThread();
     void scheduleOrDeferSyncTimer();
     void syncTimerFired(Timer<IconDatabase>*);
-
-    void updateIconRecord(PassRefPtr<SharedBuffer>, PassRefPtr<Image>, const String&);
 
     Timer<IconDatabase> m_syncTimer;
     ThreadIdentifier m_syncThread;

Source/WebCore/loader/icon/IconDatabaseBase.h

@@ -26,7 +26,6 @@
 #ifndef IconDatabaseBase_h
 #define IconDatabaseBase_h
 
-#include "Image.h"
 #include "ImageSource.h"
 #include "SharedBuffer.h"
 
@@ -173,7 +172,6 @@ class IconDatabaseBase {
 
     virtual void setIconURLForPageURL(const String&, const String&) { }
     virtual void setIconDataForIconURL(PassRefPtr<SharedBuffer>, const String&) { }
-    virtual void setIconBitmapForIconURL(PassRefPtr<Image>, const String&) { }
 
     // Synchronous calls used internally by WebCore.
     // Usage should be replaced by asynchronous calls.

Source/WebCore/loader/icon/IconRecord.cpp

@@ -77,12 +77,6 @@ void IconRecord::setImageData(PassRefPtr<SharedBuffer> data)
     m_dataSet = true;
 }
 
-void IconRecord::setImage(PassRefPtr<Image> image)
-{
-    m_image = image;
-    m_dataSet = true;
-}
-
 void IconRecord::loadImageFromResource(const char* resource)
 {
     if (!resource)

Source/WebCore/loader/icon/IconRecord.h

@@ -85,7 +85,6 @@ class IconRecord : public RefCounted<IconRecord> {
     void setTimestamp(time_t stamp) { m_stamp = stamp; }
 
     void setImageData(PassRefPtr<SharedBuffer> data);
-    void setImage(PassRefPtr<Image> data);
     Image* image(const IntSize&);    
 
     String iconURL() { return m_iconURL; }

Source/WebKit2/ChangeLog

@@ -1,3 +1,26 @@
+2013-04-11  Commit Queue  <rniwa@webkit.org>
+
+        Unreviewed, rolling out r148034, r148052, r148097, and
+        r148194.
+        http://trac.webkit.org/changeset/148034
+        http://trac.webkit.org/changeset/148052
+        http://trac.webkit.org/changeset/148097
+        http://trac.webkit.org/changeset/148194
+        https://bugs.webkit.org/show_bug.cgi?id=114463
+
+        broke mutiresolution favicons, among other things (Requested
+        by thorton on #webkit).
+
+        * UIProcess/WebIconDatabase.cpp:
+        (WebKit::WebIconDatabase::setIconDataForIconURL):
+        * UIProcess/WebIconDatabase.h:
+        (WebIconDatabase):
+        * UIProcess/WebIconDatabase.messages.in:
+        * WebProcess/IconDatabase/WebIconDatabaseProxy.cpp:
+        (WebKit::WebIconDatabaseProxy::setIconDataForIconURL):
+        * WebProcess/Plugins/PDF/PDFPlugin.mm:
+        (WebKit::PDFPlugin::destroy):
+
 2013-04-11  Anders Carlsson  <andersca@apple.com>
 
         Implement StorageManager::getValues

Source/WebKit2/UIProcess/WebIconDatabase.cpp

@@ -121,19 +121,14 @@ void WebIconDatabase::setIconURLForPageURL(const String& iconURL, const String&
         m_iconDatabaseImpl->setIconURLForPageURL(iconURL, pageURL);
 }
 
-void WebIconDatabase::setIconBitmapForIconURL(const ShareableBitmap::Handle& bitmapHandle, const String& iconURL)
+void WebIconDatabase::setIconDataForIconURL(const CoreIPC::DataReference& iconData, const String& iconURL)
 {
-    LOG(IconDatabase, "WK2 UIProcess setting icon bitmap for page URL %s", iconURL.ascii().data());
+    LOG(IconDatabase, "WK2 UIProcess setting icon data (%i bytes) for page URL %s", (int)iconData.size(), iconURL.ascii().data());
     if (!m_iconDatabaseImpl)
         return;
 
-    if (bitmapHandle.isNull()) {
-        m_iconDatabaseImpl->setIconBitmapForIconURL(0, iconURL);
-        return;
-    }
-
-    RefPtr<ShareableBitmap> iconBitmap = ShareableBitmap::create(bitmapHandle, SharedMemory::ReadOnly);
-    m_iconDatabaseImpl->setIconBitmapForIconURL(iconBitmap->createImage(), iconURL);
+    RefPtr<SharedBuffer> buffer = SharedBuffer::create(iconData.data(), iconData.size());
+    m_iconDatabaseImpl->setIconDataForIconURL(buffer.release(), iconURL);
 }
 
 void WebIconDatabase::synchronousIconDataForPageURL(const String&, CoreIPC::DataReference& iconData)

Source/WebKit2/UIProcess/WebIconDatabase.h

@@ -29,7 +29,6 @@
 #include "APIObject.h"
 
 #include "Connection.h"
-#include "ShareableBitmap.h"
 #include "WebIconDatabaseClient.h"
 #include <WebCore/IconDatabaseClient.h>
 #include <WebCore/ImageSource.h>
@@ -67,7 +66,7 @@ class WebIconDatabase : public TypedAPIObject<APIObject::TypeIconDatabase>, publ
     void retainIconForPageURL(const String&);
     void releaseIconForPageURL(const String&);
     void setIconURLForPageURL(const String&, const String&);
-    void setIconBitmapForIconURL(const ShareableBitmap::Handle&, const String&);
+    void setIconDataForIconURL(const CoreIPC::DataReference&, const String&);
 
     void synchronousIconDataForPageURL(const String&, CoreIPC::DataReference&);
     void synchronousIconURLForPageURL(const String&, String&);

Source/WebKit2/UIProcess/WebIconDatabase.messages.in

@@ -24,7 +24,7 @@ messages -> WebIconDatabase {
     RetainIconForPageURL(WTF::String pageURL)
     ReleaseIconForPageURL(WTF::String pageURL)
     SetIconURLForPageURL(WTF::String iconURL, WTF::String pageURL)
-    SetIconBitmapForIconURL(WebKit::ShareableBitmap::Handle bitmapHandle, WTF::String iconURL)
+    SetIconDataForIconURL(CoreIPC::DataReference iconData, WTF::String iconURL)
 
     SynchronousIconDataForPageURL(WTF::String pageURL) -> (CoreIPC::DataReference iconData)
     SynchronousIconURLForPageURL(WTF::String pageURL) -> (WTF::String iconURL)

Source/WebKit2/WebProcess/IconDatabase/WebIconDatabaseProxy.cpp

@@ -30,9 +30,6 @@
 #include "WebIconDatabaseMessages.h"
 #include "WebIconDatabaseProxyMessages.h"
 #include "WebProcess.h"
-#include <WebCore/BitmapImage.h>
-#include <WebCore/GraphicsContext.h>
-#include <WebCore/IntPoint.h>
 #include <WebCore/SharedBuffer.h>
 #include <wtf/text/WTFString.h>
 
@@ -138,18 +135,8 @@ void WebIconDatabaseProxy::setIconURLForPageURL(const String& iconURL, const Str
 
 void WebIconDatabaseProxy::setIconDataForIconURL(PassRefPtr<SharedBuffer> iconData, const String& iconURL)
 {
-    RefPtr<Image> image = BitmapImage::create();
-    ShareableBitmap::Handle handle;
-
-    if (image->setData(iconData, true) && !image->size().isEmpty()) {
-        RefPtr<ShareableBitmap> shareableBitmap = ShareableBitmap::createShareable(image->size(), ShareableBitmap::SupportsAlpha);
-        OwnPtr<GraphicsContext> bitmapContext = shareableBitmap->createGraphicsContext();
-        bitmapContext->drawImage(image.get(), ColorSpaceDeviceRGB, IntPoint());
-
-        shareableBitmap->createHandle(handle, SharedMemory::ReadOnly);
-    }
-
-    m_process->connection()->send(Messages::WebIconDatabase::SetIconBitmapForIconURL(handle, iconURL), 0);
+    CoreIPC::DataReference data(reinterpret_cast<const uint8_t*>(iconData ? iconData->data() : 0), iconData ? iconData->size() : 0);
+    m_process->connection()->send(Messages::WebIconDatabase::SetIconDataForIconURL(data, iconURL), 0);
 }
 
 void WebIconDatabaseProxy::urlImportFinished()

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

@@ -345,7 +345,7 @@ - (void)pdfLayerController:(PDFLayerController *)pdfLayerController didChangeSel
 {
     m_pdfLayerController.get().delegate = 0;
 
-    if (webFrame() && webFrame()->coreFrame()) {
+    if (webFrame()) {
         if (FrameView* frameView = webFrame()->coreFrame()->view())
             frameView->removeScrollableArea(this);
     }