Make it so that allow_js_access: false can be used with background pages created by window.open.

chrome/common/extensions/extension.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/common/extensions/extension.h"
 
 #include <algorithm>
 
 #include "base/base64.h"
 #include "base/basictypes.h"
@@ skipping 1810 matching lines @@
   Value* allow_js_access = NULL;
   if (!manifest_->Get(keys::kBackgroundAllowJsAccess, &allow_js_access))
     return true;
 
   if (!allow_js_access->IsType(Value::TYPE_BOOLEAN) ||
       !allow_js_access->GetAsBoolean(&allow_background_js_access_)) {
     *error = ASCIIToUTF16(errors::kInvalidBackgroundAllowJsAccess);
     return false;
   }
 
-  if (!has_background_page()) {
-    *error = ASCIIToUTF16(errors::kInvalidBackgroundAllowJsAccessNoPage);

[Charlie Reis, 2012/03/26 17:56:51]

Normally, we expect people to set allow_js_access to false, since it defaults to
true.

However, what if the manifest explicitly sets allow_js_access to true, without
specifying a background page?  Are we ok with the app creating background pages
manually and scripting them?  (Note that they will have process-per-site in that
case.)

If so, do we also allow an empty "background" block (with no mention of page or
allow_js_access) in the manifest to get this behavior?  Or should we keep it an
error for allow_js_access to be true without specifying a background page?

[Mihai Parparita -not on Chrome, 2012/03/27 01:16:07]

Not quite sure I understand this. Being able to create a background page not
tied to the "background" section in the manifest,  but instead to the
"background" permission. Apps today can already create same-process background
pages and script them, and I don't think we can change that.

[Charlie Reis, 2012/03/27 23:19:27]

Here's what I meant.  Suppose the manifest looked like this:
  "permissions": ["background"],
  "background": {
    "allow_js_access": true
  }

That's equivalent to:
  "permissions": ["background"],
  "background": {
  }

I think this used to be illegal syntax, but now we appear to be saying it's ok. 
I just wanted to confirm that it's legal.  (It means we don't create a
background page for the app until it calls window.open for one, but when we do,
it stays in process and can be scripted.  Seems ok to me, but I think it wasn't
a case we supported before.)

[Mihai Parparita -not on Chrome, 2012/03/28 20:29:01]

I'm pretty sure this was legal, and that manifest does load in M18. We never
check for the presence/absence of the background section itself, just keys
within it (kBackgroundPage, kBackgroundScripts, etc.).
This how window.open(..., "background") has worked since it was introduced in
Chrome 10 (same-process, scriptable).

-    return false;
-  }
-
   return true;
 }
 
 bool Extension::LoadWebIntentAction(const std::string& action_name,
                                     const DictionaryValue& intent_service,
                                     string16* error) {
   DCHECK(error);
   webkit_glue::WebIntentServiceData service;
   std::string value;
 
@@ skipping 1684 matching lines @@
     already_disabled(false),
     extension(extension) {}
 
 UpdatedExtensionPermissionsInfo::UpdatedExtensionPermissionsInfo(
     const Extension* extension,
     const ExtensionPermissionSet* permissions,
     Reason reason)
     : reason(reason),
       extension(extension),
       permissions(permissions) {}