Added background page loader component extension.

Added background page loader component extension.

BUG=141677
TEST=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=151923

[Mihai Parparita -not on Chrome, 2012-08-10 17:11:02]

https://chromiumcodereview.appspot.com/10832240/diff/6/chrome/browser/resourc...
File chrome/browser/resources/chromeos/backloader/manifest.json (right):

https://chromiumcodereview.appspot.com/10832240/diff/6/chrome/browser/resourc...
chrome/browser/resources/chromeos/backloader/manifest.json:8: "persistent":
false,
This is the only key that's necessary. If you see any docs that tell you to use
"transient": true, can you point them out so they can be fixed?

[zel, 2012-08-16 01:13:44]

https://chromiumcodereview.appspot.com/10832240/diff/6/chrome/browser/resourc...
File chrome/browser/resources/chromeos/backloader/manifest.json (right):

https://chromiumcodereview.appspot.com/10832240/diff/6/chrome/browser/resourc...
chrome/browser/resources/chromeos/backloader/manifest.json:8: "persistent":
false,
On 2012/08/10 17:11:02, Mihai Parparita wrote:
> This is the only key that's necessary. If you see any docs that tell you to
use
> "transient": true, can you point them out so they can be fixed?

Done.

[xiyuan, 2012-08-16 16:55:38]

http://codereview.chromium.org/10832240/diff/6/chrome/browser/resources/chrom...
File chrome/browser/resources/chromeos/backloader/manifest.json (right):

http://codereview.chromium.org/10832240/diff/6/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/backloader/manifest.json:8: "persistent":
false,
On 2012/08/16 01:13:45, zel wrote:
> On 2012/08/10 17:11:02, Mihai Parparita wrote:
> > This is the only key that's necessary. If you see any docs that tell you to
> use
> > "transient": true, can you point them out so they can be fixed?
> 
> Done.

Mihai seems to mean to keep "persistent" and remove "transient", right?

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/backloader/manifest.json (right):

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/manifest.json:2: //
chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/
nit: Could you update this url as well?

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/backloader/scripts/background.js (right):

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/scripts/background.js:17:
Loader.instance_ = new Loader(g_pages);
Could you add a warning check to ensure g_pages is defined when running here?
e.g.

  if (!Loader.instance_) {
    if (!g_pages)
      console.log('Warning: Bad g_pages.');
    Loader.instance_ = new Loader(g_pages);
  }

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/scripts/background.js:54: var msg =
{
nit: could you use a different var name here, e.g. reply?

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/scripts/background.js:93:
Loader.getInstance().initialize();
Could you wrap this into 

document.addEventListener('DOMContentLoaded', function(){
  Loader.getInstance().initialize();
});

so that we don't have a load dependency on page.js.

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/backloader/scripts/pages.js (right):

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/scripts/pages.js:7:
'http://www.google.com',
nit: 4-space indent?

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/backloader/web/cros_validator.js (right):

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/web/cros_validator.js:18:
callback_: null,
nit: prefer to use undefined

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/web/cros_validator.js:28:
document.addEventListener('DOMContentLoaded', this.onPageLoad.bind(this));
This really depends on how ChromeOSValidator is integrated with the web. If the
web starts its work on DOMContentLoaded event, this might not work.

It seems that we could just move the logic in onPageLoad function here. And it
should work either way, i.e. no matter ChromeOSValidator.initialize is called in
a DOMContentLoaded handler or directly inlined in global scope.

[zel, 2012-08-16 17:17:21]

http://codereview.chromium.org/10832240/diff/6/chrome/browser/resources/chrom...
File chrome/browser/resources/chromeos/backloader/manifest.json (right):

http://codereview.chromium.org/10832240/diff/6/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/backloader/manifest.json:8: "persistent":
false,
On 2012/08/16 16:55:38, xiyuan wrote:
> On 2012/08/16 01:13:45, zel wrote:
> > On 2012/08/10 17:11:02, Mihai Parparita wrote:
> > > This is the only key that's necessary. If you see any docs that tell you
to
> > use
> > > "transient": true, can you point them out so they can be fixed?
> > 
> > Done.
> 
> Mihai seems to mean to keep "persistent" and remove "transient", right?

Done.

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/backloader/manifest.json (right):

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/manifest.json:2: //
chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/
On 2012/08/16 16:55:38, xiyuan wrote:
> nit: Could you update this url as well?

Done.

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/backloader/scripts/background.js (right):

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/scripts/background.js:17:
Loader.instance_ = new Loader(g_pages);
On 2012/08/16 16:55:38, xiyuan wrote:
> Could you add a warning check to ensure g_pages is defined when running here?
> e.g.
> 
>   if (!Loader.instance_) {
>     if (!g_pages)
>       console.log('Warning: Bad g_pages.');
>     Loader.instance_ = new Loader(g_pages);
>   }

Done.

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/scripts/background.js:54: var msg =
{
On 2012/08/16 16:55:38, xiyuan wrote:
> nit: could you use a different var name here, e.g. reply?

Done.

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/backloader/web/cros_validator.js (right):

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/web/cros_validator.js:18:
callback_: null,
On 2012/08/16 16:55:38, xiyuan wrote:
> nit: prefer to use undefined

Done.

http://codereview.chromium.org/10832240/diff/6001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/backloader/web/cros_validator.js:28:
document.addEventListener('DOMContentLoaded', this.onPageLoad.bind(this));
On 2012/08/16 16:55:38, xiyuan wrote:
> This really depends on how ChromeOSValidator is integrated with the web. If
the
> web starts its work on DOMContentLoaded event, this might not work.
> 
> It seems that we could just move the logic in onPageLoad function here. And it
> should work either way, i.e. no matter ChromeOSValidator.initialize is called
in
> a DOMContentLoaded handler or directly inlined in global scope.

Done.

[xiyuan, 2012-08-16 17:27:18]

LGTM