WebSQL: Use url::Origin rather than database identifiers for IPC

content/browser/renderer_host/database_message_filter.cc

Index: content/browser/renderer_host/database_message_filter.cc
diff --git a/content/browser/renderer_host/database_message_filter.cc b/content/browser/renderer_host/database_message_filter.cc
index 98d930629a177c2e3af031b6daa5ea0db531228e..ab3eecc3001ef5753541feb9cb6987b8f4ab401c 100644
--- a/content/browser/renderer_host/database_message_filter.cc
+++ b/content/browser/renderer_host/database_message_filter.cc
@@ -25,6 +25,7 @@
 #include "storage/browser/quota/quota_manager_proxy.h"
 #include "storage/common/database/database_identifier.h"
 #include "third_party/sqlite/sqlite3.h"
+#include "url/origin.h"
 
 #if defined(OS_POSIX)
 #include "base/file_descriptor_posix.h"
@@ -42,6 +43,10 @@ namespace {
 const int kNumDeleteRetries = 2;
 const int kDelayDeleteRetryMs = 100;
 
+bool IsOriginValid(const url::Origin& origin) {
+  return !origin.unique();
+}
+
 }  // namespace
 
 DatabaseMessageFilter::DatabaseMessageFilter(
@@ -250,10 +255,15 @@ void DatabaseMessageFilter::OnDatabaseGetFileSize(
 }
 
 void DatabaseMessageFilter::OnDatabaseGetSpaceAvailable(
-    const std::string& origin_identifier, IPC::Message* reply_msg) {
+    const url::Origin& origin, IPC::Message* reply_msg) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(db_tracker_->quota_manager_proxy());
 
+  if (!IsOriginValid(origin)) {
+    bad_message::ReceivedBadMessage(this, bad_message::DBMF_INVALID_ORIGIN);
+    return;
+  }
+
   QuotaManager* quota_manager =
       db_tracker_->quota_manager_proxy()->quota_manager();
   if (!quota_manager) {
@@ -268,7 +278,7 @@ void DatabaseMessageFilter::OnDatabaseGetSpaceAvailable(
   TRACE_EVENT0("io", "DatabaseMessageFilter::OnDatabaseGetSpaceAvailable");
 
   quota_manager->GetUsageAndQuota(
-      storage::GetOriginFromIdentifier(origin_identifier),
+      GURL(origin.Serialize()),
       storage::kStorageTypeTemporary,
       base::Bind(
           &DatabaseMessageFilter::OnDatabaseGetUsageAndQuota, this, reply_msg));
@@ -299,36 +309,43 @@ void DatabaseMessageFilter::OnDatabaseSetFileSize(
 }
 
 void DatabaseMessageFilter::OnDatabaseOpened(
-    const std::string& origin_identifier,
+    const url::Origin& origin,
     const base::string16& database_name,
     const base::string16& description,
     int64_t estimated_size) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  if (!storage::IsValidOriginIdentifier(origin_identifier)) {
-    bad_message::ReceivedBadMessage(this,
-                                    bad_message::DBMF_INVALID_ORIGIN_ON_OPEN);

[michaeln, 2016/04/06 20:55:14]

is there a way to mark DBMF_INVALID_ORIGIN_ON_OPEN as obsolete in the .xml file?

[jsbell, 2016/04/06 23:28:22]

I went the other direction and reverted to emitting DBMF_INVALID_ORIGIN_ON_OPEN
here and adding distinct DBMFs everywhere.

Which is a question for the histograms reviewer - how much benefit do we get out
of these being distinct?

[Mark P, 2016/04/07 21:40:41]

Well, I think the benefit only comes when there are problems.  Suppose you see a
spike in some Chrome release of the bundle DBMF_INVALID_ORIGIN?  Would you wish
that you had the distinct sub-categories as entries in order to help you
investigate?

+  if (!IsOriginValid(origin)) {
+    bad_message::ReceivedBadMessage(this, bad_message::DBMF_INVALID_ORIGIN);
     return;
   }
 
-  UMA_HISTOGRAM_BOOLEAN(
-      "websql.OpenDatabase",
-      IsOriginSecure(storage::GetOriginFromIdentifier(origin_identifier)));
+  GURL origin_url(origin.Serialize());
+  UMA_HISTOGRAM_BOOLEAN("websql.OpenDatabase", IsOriginSecure(origin_url));
 
   int64_t database_size = 0;
+  std::string origin_identifier(storage::GetIdentifierFromOrigin(origin_url));
   db_tracker_->DatabaseOpened(origin_identifier, database_name, description,
                               estimated_size, &database_size);
+
   database_connections_.AddConnection(origin_identifier, database_name);
-  Send(new DatabaseMsg_UpdateSize(origin_identifier, database_name,
-                                  database_size));
+  Send(new DatabaseMsg_UpdateSize(origin, database_name, database_size));
 }
 
 void DatabaseMessageFilter::OnDatabaseModified(
-    const std::string& origin_identifier,
+    const url::Origin& origin,
     const base::string16& database_name) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  if (!database_connections_.IsDatabaseOpened(
-          origin_identifier, database_name)) {
+
+  if (!IsOriginValid(origin)) {
+    bad_message::ReceivedBadMessage(this, bad_message::DBMF_INVALID_ORIGIN);
+    return;
+  }
+
+  std::string origin_identifier(
+      storage::GetIdentifierFromOrigin(GURL(origin.Serialize())));
+  if (!database_connections_.IsDatabaseOpened(origin_identifier,
+                                              database_name)) {
     bad_message::ReceivedBadMessage(this,
                                     bad_message::DBMF_DB_NOT_OPEN_ON_MODIFY);
     return;
@@ -338,9 +355,17 @@ void DatabaseMessageFilter::OnDatabaseModified(
 }
 
 void DatabaseMessageFilter::OnDatabaseClosed(
-    const std::string& origin_identifier,
+    const url::Origin& origin,
     const base::string16& database_name) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+
+  if (!IsOriginValid(origin)) {
+    bad_message::ReceivedBadMessage(this, bad_message::DBMF_INVALID_ORIGIN);
+    return;
+  }
+
+  std::string origin_identifier(
+      storage::GetIdentifierFromOrigin(GURL(origin.Serialize())));
   if (!database_connections_.IsDatabaseOpened(
           origin_identifier, database_name)) {
     bad_message::ReceivedBadMessage(this,
@@ -353,17 +378,17 @@ void DatabaseMessageFilter::OnDatabaseClosed(
 }
 
 void DatabaseMessageFilter::OnHandleSqliteError(
-    const std::string& origin_identifier,
+    const url::Origin& origin,
     const base::string16& database_name,
     int error) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  if (!storage::IsValidOriginIdentifier(origin_identifier)) {
-    bad_message::ReceivedBadMessage(
-        this, bad_message::DBMF_INVALID_ORIGIN_ON_SQLITE_ERROR);
+  if (!IsOriginValid(origin)) {
+    bad_message::ReceivedBadMessage(this, bad_message::DBMF_INVALID_ORIGIN);
     return;
   }
-
-  db_tracker_->HandleSqliteError(origin_identifier, database_name, error);
+  db_tracker_->HandleSqliteError(
+      storage::GetIdentifierFromOrigin(GURL(origin.Serialize())), database_name,
+      error);
 }
 
 void DatabaseMessageFilter::OnDatabaseSizeChanged(
@@ -372,8 +397,9 @@ void DatabaseMessageFilter::OnDatabaseSizeChanged(
     int64_t database_size) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   if (database_connections_.IsOriginUsed(origin_identifier)) {
-    Send(new DatabaseMsg_UpdateSize(origin_identifier, database_name,
-                                    database_size));
+    Send(new DatabaseMsg_UpdateSize(
+        url::Origin(storage::GetOriginFromIdentifier(origin_identifier)),
+        database_name, database_size));
   }
 }
 
@@ -381,7 +407,9 @@ void DatabaseMessageFilter::OnDatabaseScheduledForDeletion(
     const std::string& origin_identifier,
     const base::string16& database_name) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  Send(new DatabaseMsg_CloseImmediately(origin_identifier, database_name));
+  Send(new DatabaseMsg_CloseImmediately(
+      url::Origin(storage::GetOriginFromIdentifier(origin_identifier)),
+      database_name));
 }
 
 }  // namespace content