Allow Gmail attachments to be viewed.

When displaying attachments, WebKit calls downloadFile() on an
Android-specific URL. This was crashing because it expected to
have a URLRequest available; however, there's a different code
path in WebRequest.cpp for Android URLs that doesn't create a
URLRequest.

This CL hoists the implementation of downloadFile() into
WebUrlLoaderClient, and gets the necessary headers from the
WebResponse object in didReceiveResponse (which should always
be called before downloadFile).

Gmail appends the MIME type to the URL after a ? separator,
and we have to extract this to display the file correctly.

Tests:
- Can display .txt attachments in Gmail (now fixed)
- Can download .apk files (checking for regressions)

Change-Id: I966220977972da7a2de29e663d7781b1de0e6fd4

diff --git a/WebKit/android/WebCoreSupport/WebRequest.cpp b/WebKit/android/WebCoreSupport/WebRequest.cpp
index cefd541..307a007 100644 (file)
--- a/WebKit/android/WebCoreSupport/WebRequest.cpp
+++ b/WebKit/android/WebCoreSupport/WebRequest.cpp
@@ -99,6 +99,16 @@ WebRequest::~WebRequest()
         env->DeleteGlobalRef((_jobject*)m_inputStream);
 }
 
+const std::string& WebRequest::getUrl() const
+{
+    return m_url;
+}
+
+const std::string& WebRequest::getUserAgent() const
+{
+    return m_userAgent;
+}
+
 void WebRequest::finish(bool success)
 {
     ASSERT(m_loadState < Finished, "called finish on an already finished WebRequest (%d)", m_loadState);
@@ -181,9 +191,20 @@ void WebRequest::handleAndroidURL()
     }
 
     m_loadState = Response;
-    FilePath path(m_url);
-    std::string mimeType("");
-    net::GetMimeTypeFromFile(path, &mimeType);
+
+    // Get the MIME type from the URL. "text/html" is a last resort, hopefully overridden.
+    std::string mimeType("text/html");
+
+    // Gmail appends the MIME to the end of the URL, with a ? separator.
+    size_t mimeTypeIndex = m_url.find_last_of('?');
+    if (mimeTypeIndex != std::string::npos) {
+        mimeType.assign(m_url.begin() + mimeTypeIndex + 1, m_url.end());
+    } else {
+        // Get the MIME type from the file extension, if any.
+        FilePath path(m_url);
+        net::GetMimeTypeFromFile(path, &mimeType);
+    }
+
     OwnPtr<WebResponse> webResponse(new WebResponse(m_url, mimeType, 0, "", 200));
     m_urlLoader->maybeCallOnMainThread(NewRunnableMethod(
             m_urlLoader.get(), &WebUrlLoaderClient::didReceiveResponse, webResponse.release()));
@@ -341,17 +362,6 @@ void WebRequest::cancelAuth()
     m_request->CancelAuth();
 }
 
-void WebRequest::downloadFile(WebFrame* frame)
-{
-    std::string contentDisposition;
-    std::string mimeType;
-
-    m_request->GetResponseHeaderByName("content-disposition", &contentDisposition);
-    m_request->GetMimeType(&mimeType);
-
-    frame->downloadStart(m_request->url().spec(), m_userAgent, contentDisposition, mimeType, m_request->GetExpectedContentSize());
-}
-
 void WebRequest::startReading()
 {
     ASSERT(m_loadState == Response || m_loadState == GotData, "StartReading in state other than RESPONSE and GOTDATA");

diff --git a/WebKit/android/WebCoreSupport/WebRequest.h b/WebKit/android/WebCoreSupport/WebRequest.h
index b96126b..eff39d8 100644 (file)
--- a/WebKit/android/WebCoreSupport/WebRequest.h
+++ b/WebKit/android/WebCoreSupport/WebRequest.h
@@ -74,7 +74,9 @@ public:
     // Methods called during a request by the UI code (via WebUrlLoaderClient).
     void setAuth(const string16& username, const string16& password);
     void cancelAuth();
-    void downloadFile(WebFrame*);
+
+    const std::string& getUrl() const;
+    const std::string& getUserAgent() const;
 
 private:
     void startReading();

diff --git a/WebKit/android/WebCoreSupport/WebResponse.cpp b/WebKit/android/WebCoreSupport/WebResponse.cpp
index cb10cba..5c9b79b 100644 (file)
--- a/WebKit/android/WebCoreSupport/WebResponse.cpp
+++ b/WebKit/android/WebCoreSupport/WebResponse.cpp
@@ -29,6 +29,8 @@
 #include "ResourceResponse.h"
 #include "ResourceError.h"
 
+using namespace std;
+
 namespace android {
 
 WebResponse::WebResponse(URLRequest* request)
@@ -38,7 +40,7 @@ WebResponse::WebResponse(URLRequest* request)
     m_host = request->url().HostNoBrackets();
     request->GetMimeType(&m_mime);
     request->GetCharset(&m_encoding);
-    m_length = request->GetExpectedContentSize();
+    m_expectedSize = request->GetExpectedContentSize();
 
     net::HttpResponseHeaders* responseHeaders = request->response_headers();
     if (!responseHeaders)
@@ -47,18 +49,18 @@ WebResponse::WebResponse(URLRequest* request)
     m_httpStatusCode = responseHeaders->response_code();
     m_httpStatusText = responseHeaders->GetStatusText();
 
-    std::string value;
-    std::string name;
+    string value;
+    string name;
     void* iter = 0;
     while (responseHeaders->EnumerateHeaderLines(&iter, &name, &value))
         m_headerFields[name] = value;
 }
 
-WebResponse::WebResponse(const std::string &url, const std::string &mimeType, const long long length, const std::string &encoding, const int httpStatusCode)
+WebResponse::WebResponse(const string &url, const string &mimeType, long long expectedSize, const string &encoding, int httpStatusCode)
     : m_encoding(encoding)
     , m_httpStatusCode(httpStatusCode)
     , m_httpStatusText("")
-    , m_length(length)
+    , m_expectedSize(expectedSize)
     , m_mime(mimeType)
     , m_url(url)
 {
@@ -66,11 +68,11 @@ WebResponse::WebResponse(const std::string &url, const std::string &mimeType, co
 
 WebCore::ResourceResponse WebResponse::createResourceResponse()
 {
-    WebCore::ResourceResponse resourceResponse(url(), m_mime.c_str(), m_length, m_encoding.c_str(), "");
+    WebCore::ResourceResponse resourceResponse(createKurl(), m_mime.c_str(), m_expectedSize, m_encoding.c_str(), "");
     resourceResponse.setHTTPStatusCode(m_httpStatusCode);
     resourceResponse.setHTTPStatusText(m_httpStatusText.c_str());
 
-    std::map<std::string, std::string>::const_iterator it;
+    map<string, string>::const_iterator it;
     for (it = m_headerFields.begin(); it != m_headerFields.end(); ++it)
         resourceResponse.setHTTPHeaderField(it->first.c_str(), it->second.c_str());
 
@@ -85,15 +87,40 @@ WebCore::ResourceError WebResponse::createResourceError()
 }
 
 
-WebCore::KURL WebResponse::url()
+WebCore::KURL WebResponse::createKurl()
 {
     WebCore::KURL kurl(WebCore::ParsedURLString, m_url.c_str());
     return kurl;
 }
 
-void WebResponse::setUrl(std::string url)
+const string& WebResponse::getUrl() const
+{
+    return m_url;
+}
+
+void WebResponse::setUrl(const string& url)
 {
     m_url = url;
 }
 
+const string& WebResponse::getMimeType() const
+{
+    return m_mime;
+}
+
+bool WebResponse::getHeader(const string& header, string* result) const
+{
+    map<string, string>::const_iterator iter = m_headerFields.find(header);
+    if (iter == m_headerFields.end())
+        return false;
+    if (result)
+        *result = iter->second;
+    return true;
+}
+
+long long WebResponse::getExpectedSize() const
+{
+    return m_expectedSize;
+}
+
 } // namespace android

diff --git a/WebKit/android/WebCoreSupport/WebResponse.h b/WebKit/android/WebCoreSupport/WebResponse.h
index fc90bf1..eafc4e3 100644 (file)
--- a/WebKit/android/WebCoreSupport/WebResponse.h
+++ b/WebKit/android/WebCoreSupport/WebResponse.h
@@ -46,13 +46,18 @@ class WebResponse {
 public:
     WebResponse() {}
     WebResponse(URLRequest*);
-    WebResponse(const std::string &url, const std::string &mimeType, const long long length, const std::string &encoding, const int httpStatusCode);
-    WebCore::KURL url();
-    void setUrl(std::string);
+    WebResponse(const std::string &url, const std::string &mimeType, long long expectedSize, const std::string &encoding, int httpStatusCode);
 
-    // Only use on the WebCore thread!
+    const std::string& getUrl() const;
+    void setUrl(const std::string&);
+
+    const std::string& getMimeType() const;
+    bool getHeader(const std::string& header, std::string* result) const;
+    long long getExpectedSize() const;
+
+    // The create() methods create WebCore objects. They must only be called on the WebKit thread.
+    WebCore::KURL createKurl();
     WebCore::ResourceResponse createResourceResponse();
-    // Only use on the WebCore thread!
     WebCore::ResourceError createResourceError();
 
 private:
@@ -60,7 +65,7 @@ private:
     int m_httpStatusCode;
     std::string m_host;
     std::string m_httpStatusText;
-    long long m_length;
+    long long m_expectedSize;
     std::string m_mime;
     std::string m_url;
 

diff --git a/WebKit/android/WebCoreSupport/WebUrlLoaderClient.cpp b/WebKit/android/WebCoreSupport/WebUrlLoaderClient.cpp
index e6c06af..bd116c6 100644 (file)
--- a/WebKit/android/WebCoreSupport/WebUrlLoaderClient.cpp
+++ b/WebKit/android/WebCoreSupport/WebUrlLoaderClient.cpp
@@ -23,6 +23,8 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#define LOG_TAG "WebUrlLoaderClient"
+
 #include "config.h"
 #include "WebUrlLoaderClient.h"
 
@@ -171,7 +173,16 @@ bool WebUrlLoaderClient::start(bool sync, bool isPrivateBrowsing)
 
 void WebUrlLoaderClient::downloadFile()
 {
-    m_request->downloadFile(m_webFrame);
+    if (m_response) {
+        std::string contentDisposition;
+        m_response->getHeader("content-disposition", &contentDisposition);
+        m_webFrame->downloadStart(m_request->getUrl(), m_request->getUserAgent(), contentDisposition, m_response->getMimeType(), m_response->getExpectedSize());
+    } else {
+        LOGE("Unexpected call to downloadFile() before didReceiveResponse(). URL: %s", m_request->getUrl().c_str());
+        // TODO: Turn off asserts crashing before release
+        // http://b/issue?id=2951985
+        CRASH();
+    }
 }
 
 void WebUrlLoaderClient::cancel()
@@ -243,7 +254,8 @@ void WebUrlLoaderClient::didReceiveResponse(PassOwnPtr<WebResponse> webResponse)
     if (!isActive())
         return;
 
-    m_resourceHandle->client()->didReceiveResponse(m_resourceHandle.get(), webResponse->createResourceResponse());
+    m_response = webResponse;
+    m_resourceHandle->client()->didReceiveResponse(m_resourceHandle.get(), m_response->createResourceResponse());
 }
 
 void WebUrlLoaderClient::didReceiveData(scoped_refptr<net::IOBuffer> buf, int size)
@@ -290,7 +302,7 @@ void WebUrlLoaderClient::willSendRequest(PassOwnPtr<WebResponse> webResponse)
     if (!isActive())
         return;
 
-    OwnPtr<WebCore::ResourceRequest> resourceRequest(new WebCore::ResourceRequest(webResponse->url()));
+    OwnPtr<WebCore::ResourceRequest> resourceRequest(new WebCore::ResourceRequest(webResponse->createKurl()));
     m_resourceHandle->client()->willSendRequest(m_resourceHandle.get(), *resourceRequest, webResponse->createResourceResponse());
 }
 

diff --git a/WebKit/android/WebCoreSupport/WebUrlLoaderClient.h b/WebKit/android/WebCoreSupport/WebUrlLoaderClient.h
index d038489..5f2c528 100644 (file)
--- a/WebKit/android/WebCoreSupport/WebUrlLoaderClient.h
+++ b/WebKit/android/WebCoreSupport/WebUrlLoaderClient.h
@@ -104,6 +104,7 @@ private:
     volatile bool m_finished;
 
     scoped_refptr<WebRequest> m_request;
+    OwnPtr<WebResponse> m_response; // NULL until didReceiveResponse is called.
 
     // Check if a request is active
     bool isActive() const;