Fix, improve and clean WebSnap API.

This led me to a little polish in the classes using
the WebSnap facilities and to change the mechanism
the NewTabPage retrieves images from the "cache"

8 files changed, 173 insertions, 104 deletions

diff --git a/src/newtabpage.cpp b/src/newtabpage.cpp
index b709375a..7f13647b 100644
--- a/src/newtabpage.cpp
+++ b/src/newtabpage.cpp
@@ -59,7 +59,8 @@
 
 
 NewTabPage::NewTabPage(QWebFrame *frame)
-    : m_root(frame->documentElement())
+    : QObject(frame)
+    , m_root(frame->documentElement())
 {
     QString htmlFilePath = KStandardDirs::locate("data", "rekonq/htmls/home.html");
     QString imagesPath = QString("file://") + KGlobal::dirs()->findResourceDir("data", "rekonq/pics/bg.png") + QString("rekonq/pics");    
@@ -183,18 +184,18 @@ void NewTabPage::favoritesPage()
     if(urls.isEmpty())
     {
         m_root.addClass("empty");
-        m_root.setPlainText(i18n("You can add a preview by clicking the \"Add Preview\" button in the top-right corner of this page"));
+        m_root.setPlainText( i18n("You can add a preview by clicking the \"Add Preview\" button in the top-right corner of this page") );
         return;
     }
     
     for(int i=0; i < urls.count() ; ++i)
     {
-        KUrl url = urls.at(i);
+        KUrl url = KUrl( urls.at(i) );
         QWebElement prev;
         
         if(url.isEmpty())
             prev = emptyPreview(i);
-        else if(!QFile::exists(WebSnap::fileForUrl(url).toLocalFile()))
+        else if( !WebSnap::existsImage(url) )
             prev = loadingPreview(i, url);
         else
             prev = validPreview(i, url, names.at(i));
@@ -233,9 +234,13 @@ QWebElement NewTabPage::loadingPreview(int index, const KUrl &url)
     
     setupPreview(prev, index);
     showControls(prev);
-    
-    new WebSnap(url, m_root.webFrame(), index);
-    
+
+    // NOTE: we need the page frame for two reasons
+    // 1) to link to the WebPage calling the snapFinished slot
+    // 2) to "auto-destroy" snaps on tab closing :)
+    QWebFrame *frame = qobject_cast<QWebFrame *>(parent());
+    WebSnap *snap = new WebSnap(url, frame);
+    connect(snap, SIGNAL(snapDone(bool)), frame->page(), SLOT(updateImage(bool)));
     return prev;
 }
 
@@ -243,13 +248,13 @@ QWebElement NewTabPage::loadingPreview(int index, const KUrl &url)
 QWebElement NewTabPage::validPreview(int index, const KUrl &url, const QString &title)
 {
     QWebElement prev = markup(".thumbnail");
-    KUrl previewPath = WebSnap::fileForUrl(url);
+    QString previewPath = QL1S("file://") + WebSnap::imagePathFromUrl(url);
     QString iString = QVariant(index).toString();
     
-    prev.findFirst(".preview img").setAttribute("src" , previewPath.toMimeDataString());
-    prev.findFirst("a").setAttribute("href", url.toMimeDataString());
-    prev.findFirst("span a").setAttribute("href", url.toMimeDataString());
-    prev.findFirst("span a").setPlainText(checkTitle(title));
+    prev.findFirst(".preview img").setAttribute("src" , previewPath );
+    prev.findFirst("a").setAttribute("href", url.toMimeDataString() );  // NOTE ?
+    prev.findFirst("span a").setAttribute("href", url.toMimeDataString() ); // NOTE ?
+    prev.findFirst("span a").setPlainText( checkTitle(title) );
     
     setupPreview(prev, index);
     showControls(prev);
@@ -288,34 +293,35 @@ void NewTabPage::setupPreview(QWebElement e, int index)
 }
 
 
-void NewTabPage::snapFinished(int index, const KUrl &url, const QString &title)
+void NewTabPage::snapFinished()
 {
-    // Update title if necessary
-    QStringList urls = ReKonfig::previewUrls();
-    if(KUrl(urls.at(index)) == url)
-    {
-        QStringList names = ReKonfig::previewNames();
-        names.replace(index, title);
-        ReKonfig::setPreviewNames(names);
-        
-        ReKonfig::self()->writeConfig();
-    }
-    
     // Update page, but only if open
     if(m_root.document().findAll("#rekonq-newtabpage").count() == 0)
         return;
     if(m_root.findAll(".favorites").count() == 0 && m_root.findAll(".closedTabs").count() == 0)
         return;
-        
-    QWebElement prev = m_root.findFirst("#preview" + QVariant(index).toString());
-    if(KUrl(prev.findFirst("a").attribute("href")) == url)
+    
+    QStringList urls = ReKonfig::previewUrls();
+    QStringList names = ReKonfig::previewNames();
+    
+    for(int i = 0; i < urls.count(); i++)
     {
-        QWebElement newPrev = validPreview(index, url, title);
-        
-        if(m_root.findAll(".closedTabs").count() != 0)
-            hideControls(newPrev);
+        KUrl url = KUrl( urls.at(i) );
+        QString title = names.at(i);
         
-        prev.replace(newPrev);
+        if( WebSnap::existsImage(url) )
+        {
+            QWebElement prev = m_root.findFirst("#preview" + QVariant(i).toString());
+            if( KUrl(prev.findFirst("a").attribute("href")) == url )
+            {
+                QWebElement newPrev = validPreview(i, url, title);
+                
+                if(m_root.findAll(".closedTabs").count() != 0)
+                    hideControls(newPrev);
+                
+                prev.replace(newPrev);
+            }            
+        }
     }
 }
 
@@ -513,10 +519,10 @@ void NewTabPage::closedTabsPage()
         
         if(item.url.isEmpty())
             continue;
-        else if(!QFile::exists(WebSnap::fileForUrl(item.url).toLocalFile()))
-            prev = loadingPreview(i, item.url);
-        else
-            prev = validPreview(i, item.url, item.title);
+            
+        prev = WebSnap::existsImage( KUrl(item.url) )
+            ? validPreview(i, item.url, item.title)
+            : loadingPreview(i, item.url);
         
         prev.setAttribute("id", "preview" + QVariant(i).toString());
         hideControls(prev);
diff --git a/src/newtabpage.h b/src/newtabpage.h
index 6604e4df..3b63942e 100644
--- a/src/newtabpage.h
+++ b/src/newtabpage.h
@@ -58,9 +58,12 @@ public:
      * the corresponding part of the new tab page
      */
     void generate(const KUrl &url = KUrl("about:home"));
-    
-    void snapFinished(int index, const KUrl &url, const QString &title);
 
+    /**
+     * This method updates thumbs, removing loading previews
+     * and providing a real picture
+     */
+    void snapFinished();
      
 private:
     // these are the "high-level" functions to build the new tab page.
@@ -110,7 +113,6 @@ private:
     }
     
     QString checkTitle(const QString &title);
-
     
 private:
     QString m_html;
diff --git a/src/tabbar.cpp b/src/tabbar.cpp
index f8da57b1..fca3c6b1 100644
--- a/src/tabbar.cpp
+++ b/src/tabbar.cpp
@@ -173,7 +173,7 @@ void TabBar::showTabPreview(int tab)
     m_previewPopup.data()->setFixedSize(w, h);
     
     QLabel *l = new QLabel();
-    l->setPixmap( WebSnap::renderPreview( *indexedTab->page(), w, h, true) );
+    l->setPixmap( WebSnap::renderPreview( *indexedTab->page(), w, h) );
     
     m_previewPopup.data()->setView(l);
     m_previewPopup.data()->layout()->setAlignment(Qt::AlignTop);
diff --git a/src/urlbar/listitem.cpp b/src/urlbar/listitem.cpp
index a182c1a2..987d070f 100644
--- a/src/urlbar/listitem.cpp
+++ b/src/urlbar/listitem.cpp
@@ -242,12 +242,11 @@ PreviewLabel::PreviewLabel(const QString &url, int width, int height, QWidget *p
     setFixedSize(width, height);
     setFrameStyle(QFrame::StyledPanel | QFrame::Raised);
 
-    KUrl u = WebSnap::fileForUrl( QUrl(url) );
-    QString path = u.pathOrUrl();
-    if(QFile::exists(path))
+    KUrl u = KUrl(url);
+    if( WebSnap::existsImage( KUrl(u) ) )
     {     
         QPixmap preview;
-        preview.load(path);
+        preview.load( WebSnap::imagePathFromUrl(u) );
         setPixmap(preview.scaled(width, height, Qt::IgnoreAspectRatio, Qt::SmoothTransformation));
     }
 }
diff --git a/src/webpage.cpp b/src/webpage.cpp
index c3acc1ad..1e25ce8d 100644
--- a/src/webpage.cpp
+++ b/src/webpage.cpp
@@ -564,3 +564,55 @@ void WebPage::showSSLInfo()
                                 );
     }
 }
+
+
+
+
+void WebPage::updateImage(bool ok)
+{
+    if(!ok)
+        kDebug() << "OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO";
+    else
+        kDebug() << "YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY";
+    
+/*    int previewIndex = 1;
+    KUrl url;
+    QString snapTitle = QString("ciao");*/
+//     // Update page, but only if open
+//     if(m_root.document().findAll("#rekonq-newtabpage").count() == 0)
+//         return;
+//     if(m_root.findAll(".favorites").count() == 0 || m_root.findAll(".closedTabs").count() == 0)
+//         return;
+//         
+//     QStringList urls = ReKonfig::previewUrls();
+//     
+//     for(int i = 0; i < urls.count(); i++)
+//     {
+//         QString title = urls.at(i);
+//         KUrl url( title );
+//         
+//         
+//         if( WebSnap::existsImage(url) )
+//         {
+//             QStringList names = ReKonfig::previewNames();
+//             names.replace(i, title);
+//             ReKonfig::setPreviewNames(names);            
+//             ReKonfig::self()->writeConfig();
+//    
+//             QWebElement prev = m_root.findFirst("#preview" + QVariant(i).toString());
+//             if(KUrl(prev.findFirst("a").attribute("href")) == url )
+//             {
+//                 QWebElement newPrev = validPreview(i, url, title);
+//                 
+//                 if(m_root.findAll(".closedTabs").count() != 0)
+//                     hideControls(newPrev);
+//                 
+//                 prev.replace(newPrev);
+//             }
+//             break;
+//         }
+//     }
+
+    NewTabPage p( mainFrame() );
+    p.snapFinished();
+}
diff --git a/src/webpage.h b/src/webpage.h
index 9583cc22..dcd40cf6 100644
--- a/src/webpage.h
+++ b/src/webpage.h
@@ -76,7 +76,8 @@ private slots:
     void manageNetworkErrors(QNetworkReply *reply);
     void loadFinished(bool);
     void showSSLInfo();
-    
+    void updateImage(bool ok);
+
 private:
     QString errorPage(QNetworkReply *);
     
diff --git a/src/websnap.cpp b/src/websnap.cpp
index 612fdf4e..6be7314e 100644
--- a/src/websnap.cpp
+++ b/src/websnap.cpp
@@ -46,11 +46,9 @@
 #include <QFile>
 
 
-WebSnap::WebSnap(const QUrl& url, QWebFrame *frame, int index)
-    : QObject()
+WebSnap::WebSnap(const KUrl& url, QObject *parent)
+    : QObject(parent)
     , m_url(url)
-    , m_frame(frame)
-    , m_previewIndex(index)
 {
     // this to not register websnap history
     m_page.settings()->setAttribute(QWebSettings::PrivateBrowsingEnabled, true);
@@ -63,8 +61,7 @@ WebSnap::WebSnap(const QUrl& url, QWebFrame *frame, int index)
     
     QTimer::singleShot(0, this, SLOT(load()));
 }
-
-
+    
 void WebSnap::load()
 {
     m_page.mainFrame()->load(m_url);
@@ -73,7 +70,7 @@ void WebSnap::load()
 
 // NOTE please, be careful modifying this. 
 // You are playing with fire..
-QPixmap WebSnap::renderPreview(const QWebPage &page, int w, int h, bool save)
+QPixmap WebSnap::renderPreview(const QWebPage &page, int w, int h)
 {
     // prepare page
     page.mainFrame()->setScrollBarPolicy(Qt::Vertical, Qt::ScrollBarAlwaysOff);
@@ -106,28 +103,21 @@ QPixmap WebSnap::renderPreview(const QWebPage &page, int w, int h, bool save)
     page.setViewportSize(oldSize);
     
     QPixmap pm = QPixmap::fromImage(pageImage);
-    if(save)
-    {
-        KUrl url( page.mainFrame()->url() );
-        kDebug() << "saving preview";
-        QFile::remove( fileForUrl(url).toLocalFile() );
-        pm.save(fileForUrl(url).toLocalFile());
-    }
+    KUrl url( page.mainFrame()->url() );
+    kDebug() << "saving preview";
+    
+    QString path = imagePathFromUrl(url);
+    QFile::remove( path );
+    pm.save( path );
     
     return pm;
 }
 
 
-KUrl WebSnap::fileForUrl(KUrl url)
+QString WebSnap::imagePathFromUrl(const KUrl &url)
 {
-    QString filePath = KStandardDirs::locateLocal("cache", QString("thumbs/") + WebSnap::guessNameFromUrl(url) + ".png", true);
-    return KUrl(filePath);
-}
-
-
-QString WebSnap::guessNameFromUrl(QUrl url)
-{
-    QString name = url.toString( QUrl::RemoveScheme | QUrl::RemoveUserInfo | QUrl::StripTrailingSlash );
+    QUrl temp = QUrl( url.url() );
+    QString name = temp.toString( QUrl::RemoveScheme | QUrl::RemoveUserInfo | QUrl::StripTrailingSlash );
     
     // TODO learn Regular Expressions :)
     // and implement something better here..
@@ -140,37 +130,28 @@ QString WebSnap::guessNameFromUrl(QUrl url)
     name.remove('=');
     name.remove('+');
     
-    return name;
+    return KStandardDirs::locateLocal("cache", QString("thumbs/") + name + ".png", true);
 }
 
 
 void WebSnap::saveResult(bool ok)
 {
-    QPixmap image = QPixmap();
-    
-    // crude error-checking
-    if (!ok) 
+    if (ok) 
     {
-        kDebug() << "Error loading site..";
-        m_snapTitle = "Error...";
-        
+        QPixmap image = renderPreview(m_page, WIDTH, HEIGHT);
+        QString path = imagePathFromUrl( m_url);
+        QFile::remove( path );
+        image.save( path );
     }
-    else
-    {
-        image = renderPreview(m_page, WIDTH, HEIGHT);
-        m_snapTitle = m_page.mainFrame()->title();
-    }
-    QFile::remove(fileForUrl(m_url).toLocalFile());
-    image.save(fileForUrl(m_url).toLocalFile());
-    
-    NewTabPage p( m_frame );
-    p.snapFinished(m_previewIndex, m_url, m_snapTitle);
+
+    emit snapDone(ok);
+    kDebug() << "SAVE RESULTS: " << ok << " URL: " << m_url;
     
     this->deleteLater();
 }
 
 
-QString WebSnap::snapTitle()
+bool WebSnap::existsImage(const KUrl &u)
 {
-    return m_page.mainFrame()->title();
+    return QFile::exists( imagePathFromUrl(u) );
 }
diff --git a/src/websnap.h b/src/websnap.h
index 9773b4cc..c77c0dc8 100644
--- a/src/websnap.h
+++ b/src/websnap.h
@@ -53,37 +53,65 @@
  * Heavily based on Graphics-Dojo WebSnap example (thanks!)
  *
  * We use this in the following rekonq classes:
- * - TabBar class:          to show a tab preview                (given a page, you show WITHOUT saving an image)
- * - NewTabPage class:      to show the favorites page "preview" (given an url, you show AND save an image)
+ *
+ * - TabBar class:          to show a tab preview                (given a page, you show AND save an image)
  * - PreviewSelector class: to save new favorite selection       (given a page, you show AND save an image)
  *
+ * - NewTabPage class:      to show the favorites page "preview" (given an url, you show AND save an image)
+ *
  */
 class REKONQ_TESTS_EXPORT WebSnap : public QObject
 {
     Q_OBJECT
 
 public:
-    WebSnap(const QUrl &url, QWebFrame *frame, int index);
-       
-    static QPixmap renderPreview(const QWebPage &page, int w = WIDTH, int h = HEIGHT, bool save = true);
-        
-    static KUrl fileForUrl(KUrl url);
-    static QString guessNameFromUrl(QUrl url);
+    /**
+     * Creates a WebSnap object. It will load the url in one WebPage
+     * and snap an image from it.
+     *
+     * @param url the url to load
+     * @param parent the object parent
+     */
+    WebSnap(const KUrl &url, QObject *parent = 0);
+     
+    /**
+     * Snaps a pixmap of size w * h from a page and save it to cache
+     *
+     * @param page the page to snap
+     * @param w the image width
+     * @param h the image height
+     *
+     * @return the pixmap snapped from the page
+     */
+    static QPixmap renderPreview(const QWebPage &page, int w = WIDTH, int h = HEIGHT);        
+
+    /**
+     * Guess the local path where the image for the url provided
+     * should be
+     *
+     * @param url the url to guess snap path
+     *
+     * @return the local path of the url snap
+     */
+    static QString imagePathFromUrl(const KUrl &url);
     
-    QString snapTitle();
+    /**
+     * Determines if a snap exists for that url
+     *
+     */
+    static bool existsImage(const KUrl &url);
 
+    
 private slots:
-    void load();
     void saveResult(bool ok = true);
+    void load();
 
+signals:
+    void snapDone(bool ok);
+        
 private:
     QWebPage m_page;
-
-    QUrl m_url;
-    QString m_snapTitle;
-    
-    QWebFrame *m_frame;
-    int m_previewIndex;
+    KUrl m_url;
 };
 
 #endif // WEB_SNAP_H