Merge 33164 - Changed shared worker code so incognito windows do not have access to nonincognito shared workers.

BUG=27883
TEST=added new uitest

Review URL: http://codereview.chromium.org/441022

TBR=atwilson@chromium.org
Review URL: http://codereview.chromium.org/450016

git-svn-id: svn://svn.chromium.org/chrome/branches/249/src@33375 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 150 insertions, 43 deletions

diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index f4dd68f..e43f350 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -648,7 +648,8 @@ void ResourceMessageFilter::OnCreateWorker(const GURL& url,
                                            int* route_id) {
   *route_id = render_widget_helper_->GetNextRoutingID();
   WorkerService::GetInstance()->CreateWorker(
-      url, is_shared, name, id(), render_view_route_id, this, *route_id);
+      url, is_shared, off_the_record(), name, id(), render_view_route_id, this,
+      *route_id);
 }
 
 void ResourceMessageFilter::OnLookupSharedWorker(const GURL& url,
@@ -658,7 +659,8 @@ void ResourceMessageFilter::OnLookupSharedWorker(const GURL& url,
                                                  bool* url_mismatch) {
   int new_route_id = render_widget_helper_->GetNextRoutingID();
   bool worker_found = WorkerService::GetInstance()->LookupSharedWorker(
-      url, name, document_id, this, new_route_id, url_mismatch);
+      url, name, off_the_record(), document_id, this, new_route_id,
+      url_mismatch);
   *route_id = worker_found ? new_route_id : MSG_ROUTING_NONE;
 }
 
diff --git a/chrome/browser/worker_host/worker_process_host.cc b/chrome/browser/worker_host/worker_process_host.cc
index 0f6786b..e2be3ef 100644
--- a/chrome/browser/worker_host/worker_process_host.cc
+++ b/chrome/browser/worker_host/worker_process_host.cc
@@ -124,7 +124,7 @@ void WorkerProcessHost::CreateWorker(const WorkerInstance& instance) {
 
   instances_.push_back(instance);
   Send(new WorkerProcessMsg_CreateWorker(instance.url(),
-                                         instance.is_shared(),
+                                         instance.shared(),
                                          instance.name(),
                                          instance.worker_route_id()));
 
@@ -136,7 +136,7 @@ void WorkerProcessHost::CreateWorker(const WorkerInstance& instance) {
 bool WorkerProcessHost::FilterMessage(const IPC::Message& message,
                                       IPC::Message::Sender* sender) {
   for (Instances::iterator i = instances_.begin(); i != instances_.end(); ++i) {
-    if (!i->is_closed() && i->HasSender(sender, message.routing_id())) {
+    if (!i->closed() && i->HasSender(sender, message.routing_id())) {
       RelayMessage(
           message, this, i->worker_route_id(), next_route_id_callback_.get());
       return true;
@@ -196,7 +196,7 @@ void WorkerProcessHost::OnMessageReceived(const IPC::Message& message) {
 
   for (Instances::iterator i = instances_.begin(); i != instances_.end(); ++i) {
     if (i->worker_route_id() == message.routing_id()) {
-      if (!i->is_shared()) {
+      if (!i->shared()) {
         // Don't relay messages from shared workers (all communication is via
         // the message port).
         WorkerInstance::SenderInfo info = i->GetSender();
@@ -295,7 +295,7 @@ void WorkerProcessHost::SenderShutdown(IPC::Message::Sender* sender) {
   for (Instances::iterator i = instances_.begin(); i != instances_.end();) {
     bool shutdown = false;
     i->RemoveSenders(sender);
-    if (i->is_shared()) {
+    if (i->shared()) {
       i->RemoveAllAssociatedDocuments(sender);
       if (i->IsDocumentSetEmpty()) {
         shutdown = true;
@@ -338,26 +338,30 @@ void WorkerProcessHost::UpdateTitle() {
 }
 
 void WorkerProcessHost::OnLookupSharedWorker(const GURL& url,
-                                                 const string16& name,
-                                                 unsigned long long document_id,
-                                                 int* route_id,
-                                                 bool* url_mismatch) {
+                                             const string16& name,
+                                             unsigned long long document_id,
+                                             int* route_id,
+                                             bool* url_mismatch) {
   int new_route_id = WorkerService::GetInstance()->next_worker_route_id();
+  // TODO(atwilson): Add code to merge document sets for nested shared workers.
   bool worker_found = WorkerService::GetInstance()->LookupSharedWorker(
-      url, name, document_id, this, new_route_id, url_mismatch);
+      url, name, instances_.front().off_the_record(), document_id, this,
+      new_route_id, url_mismatch);
   *route_id = worker_found ? new_route_id : MSG_ROUTING_NONE;
 }
 
 void WorkerProcessHost::OnCreateWorker(const GURL& url,
-                                       bool is_shared,
+                                       bool shared,
                                        const string16& name,
                                        int render_view_route_id,
                                        int* route_id) {
   DCHECK(instances_.size() == 1);  // Only called when one process per worker.
   *route_id = WorkerService::GetInstance()->next_worker_route_id();
   WorkerService::GetInstance()->CreateWorker(
-      url, is_shared, name, instances_.front().renderer_id(),
+      url, shared, instances_.front().off_the_record(), name,
+      instances_.front().renderer_id(),
       instances_.front().render_view_route_id(), this, *route_id);
+  // TODO(atwilson): Add code to merge document sets for nested shared workers.
 }
 
 void WorkerProcessHost::OnCancelCreateDedicatedWorker(int route_id) {
@@ -373,7 +377,7 @@ void WorkerProcessHost::DocumentDetached(IPC::Message::Sender* parent,
 {
   // Walk all instances and remove the document from their document set.
   for (Instances::iterator i = instances_.begin(); i != instances_.end();) {
-    if (!i->is_shared()) {
+    if (!i->shared()) {
       ++i;
     } else {
       i->RemoveFromDocumentSet(parent, document_id);
@@ -389,13 +393,15 @@ void WorkerProcessHost::DocumentDetached(IPC::Message::Sender* parent,
 }
 
 WorkerProcessHost::WorkerInstance::WorkerInstance(const GURL& url,
-                                                  bool is_shared,
+                                                  bool shared,
+                                                  bool off_the_record,
                                                   const string16& name,
                                                   int renderer_id,
                                                   int render_view_route_id,
                                                   int worker_route_id)
     : url_(url),
-      shared_(is_shared),
+      shared_(shared),
+      off_the_record_(off_the_record),
       closed_(false),
       name_(name),
       renderer_id_(renderer_id),
@@ -409,11 +415,16 @@ WorkerProcessHost::WorkerInstance::WorkerInstance(const GURL& url,
 // -or-
 // b) the names are both empty, and the urls are equal
 bool WorkerProcessHost::WorkerInstance::Matches(
-    const GURL& match_url, const string16& match_name) const {
+    const GURL& match_url, const string16& match_name,
+    bool off_the_record) const {
   // Only match open shared workers.
   if (!shared_ || closed_)
     return false;
 
+  // Incognito workers don't match non-incognito workers.
+  if (off_the_record_ != off_the_record)
+    return false;
+
   if (url_.GetOrigin() != match_url.GetOrigin())
     return false;
 
diff --git a/chrome/browser/worker_host/worker_process_host.h b/chrome/browser/worker_host/worker_process_host.h
index ad4150b..48a59e8 100644
--- a/chrome/browser/worker_host/worker_process_host.h
+++ b/chrome/browser/worker_host/worker_process_host.h
@@ -20,7 +20,8 @@ class WorkerProcessHost : public ChildProcessHost {
   class WorkerInstance {
    public:
     WorkerInstance(const GURL& url,
-                   bool is_shared,
+                   bool shared,
+                   bool off_the_record,
                    const string16& name,
                    int renderer_id,
                    int render_view_route_id,
@@ -41,7 +42,8 @@ class WorkerProcessHost : public ChildProcessHost {
     // Checks if this WorkerInstance matches the passed url/name params
     // (per the comparison algorithm in the WebWorkers spec). This API only
     // applies to shared workers.
-    bool Matches(const GURL& url, const string16& name) const;
+    bool Matches(
+        const GURL& url, const string16& name, bool off_the_record) const;
 
     // Adds a document to a shared worker's document set.
     void AddToDocumentSet(IPC::Message::Sender* parent,
@@ -69,8 +71,9 @@ class WorkerProcessHost : public ChildProcessHost {
 
 
     // Accessors
-    bool is_shared() const { return shared_; }
-    bool is_closed() const { return closed_; }
+    bool shared() const { return shared_; }
+    bool off_the_record() const { return off_the_record_; }
+    bool closed() const { return closed_; }
     void set_closed(bool closed) { closed_ = closed; }
     const GURL& url() const { return url_; }
     const string16 name() const { return name_; }
@@ -86,6 +89,7 @@ class WorkerProcessHost : public ChildProcessHost {
     typedef std::list<SenderInfo> SenderList;
     GURL url_;
     bool shared_;
+    bool off_the_record_;
     bool closed_;
     string16 name_;
     int renderer_id_;
@@ -158,7 +162,7 @@ class WorkerProcessHost : public ChildProcessHost {
   void UpdateTitle();
 
   void OnCreateWorker(const GURL& url,
-                      bool is_shared,
+                      bool shared,
                       const string16& name,
                       int render_view_route_id,
                       int* route_id);
diff --git a/chrome/browser/worker_host/worker_service.cc b/chrome/browser/worker_host/worker_service.cc
index 404f0a7..fe768c3 100644
--- a/chrome/browser/worker_host/worker_service.cc
+++ b/chrome/browser/worker_host/worker_service.cc
@@ -47,6 +47,7 @@ WorkerService::~WorkerService() {
 
 bool WorkerService::CreateWorker(const GURL &url,
                                  bool is_shared,
+                                 bool off_the_record,
                                  const string16& name,
                                  int renderer_id,
                                  int render_view_route_id,
@@ -58,6 +59,7 @@ bool WorkerService::CreateWorker(const GURL &url,
   // it to.
   WorkerProcessHost::WorkerInstance instance(url,
                                              is_shared,
+                                             off_the_record,
                                              name,
                                              renderer_id,
                                              render_view_route_id,
@@ -83,7 +85,7 @@ bool WorkerService::CreateWorker(const GURL &url,
   if (is_shared) {
     // See if a worker with this name already exists.
     WorkerProcessHost::WorkerInstance* existing_instance =
-        FindSharedWorkerInstance(url, name);
+        FindSharedWorkerInstance(url, name, off_the_record);
     // If this worker is already running, no need to create a new copy. Just
     // inform the caller that the worker has been created.
     if (existing_instance) {
@@ -93,7 +95,8 @@ bool WorkerService::CreateWorker(const GURL &url,
     }
 
     // Look to see if there's a pending instance.
-    WorkerProcessHost::WorkerInstance* pending = FindPendingInstance(url, name);
+    WorkerProcessHost::WorkerInstance* pending = FindPendingInstance(
+        url, name, off_the_record);
     // If there's no instance *and* no pending instance, then it means the
     // worker started up and exited already. Log a warning because this should
     // be a very rare occurrence and is probably a bug, but it *can* happen so
@@ -107,7 +110,7 @@ bool WorkerService::CreateWorker(const GURL &url,
     // worker to the new instance.
     DCHECK(!pending->IsDocumentSetEmpty());
     instance.CopyDocumentSet(*pending);
-    RemovePendingInstance(url, name);
+    RemovePendingInstance(url, name, off_the_record);
   }
 
   if (!worker) {
@@ -124,13 +127,14 @@ bool WorkerService::CreateWorker(const GURL &url,
 
 bool WorkerService::LookupSharedWorker(const GURL &url,
                                        const string16& name,
+                                       bool off_the_record,
                                        unsigned long long document_id,
                                        IPC::Message::Sender* sender,
                                        int sender_route_id,
                                        bool* url_mismatch) {
   bool found_instance = true;
   WorkerProcessHost::WorkerInstance* instance =
-      FindSharedWorkerInstance(url, name);
+      FindSharedWorkerInstance(url, name, off_the_record);
 
   if (!instance) {
     // If no worker instance currently exists, we need to create a pending
@@ -138,7 +142,7 @@ bool WorkerService::LookupSharedWorker(const GURL &url,
     // mismatched URL get the appropriate url_mismatch error at lookup time.
     // Having named shared workers was a Really Bad Idea due to details like
     // this.
-    instance = CreatePendingInstance(url, name);
+    instance = CreatePendingInstance(url, name, off_the_record);
     found_instance = false;
   }
 
@@ -171,7 +175,7 @@ void WorkerService::DocumentDetached(IPC::Message::Sender* sender,
   // Remove any queued shared workers for this document.
   for (WorkerProcessHost::Instances::iterator iter = queued_workers_.begin();
        iter != queued_workers_.end();) {
-    if (iter->is_shared()) {
+    if (iter->shared()) {
       iter->RemoveFromDocumentSet(sender, document_id);
       if (iter->IsDocumentSetEmpty()) {
         iter = queued_workers_.erase(iter);
@@ -200,7 +204,7 @@ void WorkerService::CancelCreateDedicatedWorker(IPC::Message::Sender* sender,
   for (WorkerProcessHost::Instances::iterator i = queued_workers_.begin();
        i != queued_workers_.end(); ++i) {
     if (i->HasSender(sender, sender_route_id)) {
-      DCHECK(!i->is_shared());
+      DCHECK(!i->shared());
       queued_workers_.erase(i);
       return;
     }
@@ -398,7 +402,8 @@ const WorkerProcessHost::WorkerInstance* WorkerService::FindWorkerInstance(
 }
 
 WorkerProcessHost::WorkerInstance*
-WorkerService::FindSharedWorkerInstance(const GURL& url, const string16& name) {
+WorkerService::FindSharedWorkerInstance(const GURL& url, const string16& name,
+                                        bool off_the_record) {
   for (ChildProcessHost::Iterator iter(ChildProcessInfo::WORKER_PROCESS);
        !iter.Done(); ++iter) {
     WorkerProcessHost* worker = static_cast<WorkerProcessHost*>(*iter);
@@ -406,7 +411,7 @@ WorkerService::FindSharedWorkerInstance(const GURL& url, const string16& name) {
              worker->mutable_instances().begin();
          instance_iter != worker->mutable_instances().end();
          ++instance_iter) {
-      if (instance_iter->Matches(url, name))
+      if (instance_iter->Matches(url, name, off_the_record))
         return &(*instance_iter);
     }
   }
@@ -414,13 +419,14 @@ WorkerService::FindSharedWorkerInstance(const GURL& url, const string16& name) {
 }
 
 WorkerProcessHost::WorkerInstance*
-WorkerService::FindPendingInstance(const GURL& url, const string16& name) {
+WorkerService::FindPendingInstance(const GURL& url, const string16& name,
+                                   bool off_the_record) {
   // Walk the pending instances looking for a matching pending worker.
   for (WorkerProcessHost::Instances::iterator iter =
            pending_shared_workers_.begin();
        iter != pending_shared_workers_.end();
        ++iter) {
-    if (iter->Matches(url, name)) {
+    if (iter->Matches(url, name, off_the_record)) {
       return &(*iter);
     }
   }
@@ -429,13 +435,14 @@ WorkerService::FindPendingInstance(const GURL& url, const string16& name) {
 
 
 void WorkerService::RemovePendingInstance(const GURL& url,
-                                          const string16& name) {
+                                          const string16& name,
+                                          bool off_the_record) {
   // Walk the pending instances looking for a matching pending worker.
   for (WorkerProcessHost::Instances::iterator iter =
            pending_shared_workers_.begin();
        iter != pending_shared_workers_.end();
        ++iter) {
-    if (iter->Matches(url, name)) {
+    if (iter->Matches(url, name, off_the_record)) {
       pending_shared_workers_.erase(iter);
       break;
     }
@@ -444,16 +451,17 @@ void WorkerService::RemovePendingInstance(const GURL& url,
 
 WorkerProcessHost::WorkerInstance*
 WorkerService::CreatePendingInstance(const GURL& url,
-                                     const string16& name) {
+                                     const string16& name,
+                                     bool off_the_record) {
   // Look for an existing pending worker.
   WorkerProcessHost::WorkerInstance* instance =
-      FindPendingInstance(url, name);
+      FindPendingInstance(url, name, off_the_record);
   if (instance)
     return instance;
 
   // No existing pending worker - create a new one.
   WorkerProcessHost::WorkerInstance pending(
-      url, true, name, 0, MSG_ROUTING_NONE, MSG_ROUTING_NONE);
+      url, true, off_the_record, name, 0, MSG_ROUTING_NONE, MSG_ROUTING_NONE);
   pending_shared_workers_.push_back(pending);
   return &pending_shared_workers_.back();
 }
diff --git a/chrome/browser/worker_host/worker_service.h b/chrome/browser/worker_host/worker_service.h
index 0de4121..5db6eee 100644
--- a/chrome/browser/worker_host/worker_service.h
+++ b/chrome/browser/worker_host/worker_service.h
@@ -28,6 +28,7 @@ class WorkerService : public NotificationObserver {
   // Creates a dedicated worker.  Returns true on success.
   bool CreateWorker(const GURL &url,
                     bool is_shared,
+                    bool is_off_the_record,
                     const string16& name,
                     int renderer_pid,
                     int render_view_route_id,
@@ -40,6 +41,7 @@ class WorkerService : public NotificationObserver {
   // existing shared worker with the same name.
   bool LookupSharedWorker(const GURL &url,
                           const string16& name,
+                          bool off_the_record,
                           unsigned long long document_id,
                           IPC::Message::Sender* sender,
                           int sender_route_id,
@@ -67,7 +69,7 @@ class WorkerService : public NotificationObserver {
       int worker_process_id);
 
   WorkerProcessHost::WorkerInstance* FindSharedWorkerInstance(
-      const GURL& url, const string16& name);
+      const GURL& url, const string16& name, bool off_the_record);
 
   // Used when multiple workers can run in the same process.
   static const int kMaxWorkerProcessesWhenSharing;
@@ -112,10 +114,11 @@ class WorkerService : public NotificationObserver {
 
   // APIs for manipulating our set of pending shared worker instances.
   WorkerProcessHost::WorkerInstance* CreatePendingInstance(
-      const GURL& url, const string16& name);
+      const GURL& url, const string16& name, bool off_the_record);
   WorkerProcessHost::WorkerInstance* FindPendingInstance(
-      const GURL& url, const string16& name);
-  void RemovePendingInstance(const GURL& url, const string16& name);
+      const GURL& url, const string16& name, bool off_the_record);
+  void RemovePendingInstance(
+      const GURL& url, const string16& name, bool off_the_record);
 
   NotificationRegistrar registrar_;
   int next_worker_route_id_;
diff --git a/chrome/test/data/workers/incognito_worker.html b/chrome/test/data/workers/incognito_worker.html
new file mode 100644
index 0000000..d8ba3ac
--- /dev/null
+++ b/chrome/test/data/workers/incognito_worker.html
@@ -0,0 +1,34 @@
+<html>
+
+<head>
+<title>Incognito Worker Test</title>
+
+<script src="worker_utils.js"></script>
+
+<script>
+var worker = new SharedWorker("incognito_worker.js");
+// Worker should only get a single connect event.
+worker.port.onmessage = function(evt) {
+  if (evt.data != 1) {
+    // This instance should not be shared with other pre-existing instances,
+    // so the connect count should be 1.
+    onFailure();
+    return;
+  }
+  // Make a second worker, make sure it shares this instance
+  var worker = new SharedWorker("incognito_worker.js");
+  worker.port.onmessage = function(evt) {
+    if (evt.data == 2)
+      onSuccess();
+    else
+      onFailure();
+  };
+};
+
+</script>
+</head>
+
+<body>
+<div id=statusPanel></div>
+</body>
+</html>
diff --git a/chrome/test/data/workers/incognito_worker.js b/chrome/test/data/workers/incognito_worker.js
new file mode 100644
index 0000000..cb5b7bc
--- /dev/null
+++ b/chrome/test/data/workers/incognito_worker.js
@@ -0,0 +1,5 @@
+var count = 0;
+
+onconnect = function(event) {
+  event.ports[0].postMessage(++count);
+};
\ No newline at end of file
diff --git a/chrome/worker/DEPS b/chrome/worker/DEPS
index 5d7c881..194de05 100644
--- a/chrome/worker/DEPS
+++ b/chrome/worker/DEPS
@@ -1,4 +1,5 @@
 include_rules = [

+  "+chrome/app", # For UI test

   "+chrome/browser/worker_host",  # For UI test.

   "+chrome/renderer",

   "+chrome/worker",

diff --git a/chrome/worker/worker_uitest.cc b/chrome/worker/worker_uitest.cc
index cbee62b..83d9ed7 100644
--- a/chrome/worker/worker_uitest.cc
+++ b/chrome/worker/worker_uitest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/string_util.h"
+#include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/worker_host/worker_service.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/automation/browser_proxy.h"
@@ -28,6 +29,30 @@ class WorkerTest : public UILayoutTest {
     ASSERT_STREQ(kTestCompleteSuccess, value.c_str());
   }
 
+  void RunIncognitoTest(const std::wstring& test_case) {
+    scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
+    // Open an Incognito window.
+    int window_count;
+    ASSERT_TRUE(browser->RunCommand(IDC_NEW_INCOGNITO_WINDOW));
+    scoped_refptr<BrowserProxy> incognito(automation()->GetBrowserWindow(1));
+    scoped_refptr<TabProxy> tab(incognito->GetTab(0));
+    ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
+    ASSERT_EQ(2, window_count);
+
+    GURL url = GetTestUrl(L"workers", test_case);
+    ASSERT_TRUE(tab->NavigateToURL(url));
+
+    std::string value = WaitUntilCookieNonEmpty(tab.get(), url,
+        kTestCompleteCookie, kTestIntervalMs, kTestWaitTimeoutMs);
+
+    // Close the incognito window
+    ASSERT_TRUE(incognito->RunCommand(IDC_CLOSE_WINDOW));
+    ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
+    ASSERT_EQ(1, window_count);
+
+    ASSERT_STREQ(kTestCompleteSuccess, value.c_str());
+  }
+
   bool WaitForProcessCountToBe(int tabs, int workers) {
     // The 1 is for the browser process.
     int number_of_processes = 1 + workers +
@@ -59,6 +84,14 @@ TEST_F(WorkerTest, MultipleWorkers) {
   RunTest(L"multi_worker.html");
 }
 
+// Incognito windows should not share workers with non-incognito windows
+TEST_F(WorkerTest, IncognitoSharedWorkers) {
+  // Load a non-incognito tab and have it create a shared worker
+  RunTest(L"incognito_worker.html");
+  // Incognito worker should not share with non-incognito
+  RunIncognitoTest(L"incognito_worker.html");
+}
+
 #if defined(OS_LINUX)
 #define WorkerFastLayoutTests DISABLED_WorkerFastLayoutTests
 #endif
@@ -151,8 +184,14 @@ TEST_F(WorkerTest, SharedWorkerFastLayoutTests) {
   resource_dir = resource_dir.AppendASCII("resources");
   AddResourceForLayoutTest(js_dir, resource_dir);
 
-  for (size_t i = 0; i < arraysize(kLayoutTestFiles); ++i)
+  for (size_t i = 0; i < arraysize(kLayoutTestFiles); ++i) {
     RunLayoutTest(kLayoutTestFiles[i], false);
+    // Shared workers will error out if we ever have more than one tab open.
+    int window_count = 0;
+    ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
+    ASSERT_EQ(1, window_count);
+    EXPECT_EQ(1, GetTabCount());
+  }
 }
 
 TEST_F(WorkerTest, WorkerHttpLayoutTests) {