Add support to extension_shell and ash_shell to use external V8 snapshot files.

Add support to extension_shell and ash_shell to use external V8 snapshot files.

Adds support to extension_shell and ash_shell to use external V8 snapshot files
in preparation for moving ChromeOS and ChromeCast to use this.

Re-factors the chrome_content_browser_client and
content/shell_content_browser_client to allow more reuse
of the code which opens the V8 external snapshot for child processes
by adding IsolateHolder::OpenV8FilesForChildProcesses.

This does not yet switch ChromeOS to use external V8 snapshot files -
this will be done in follow up CL https://codereview.chromium.org/1019123002.

BUG=421063
Committed: https://crrev.com/54fab5e3b8f16436d3329ecbdb6c40e050278a51
Cr-Commit-Position: refs/heads/master@{#323953}

[rmcilroy, 2015-03-18 22:03:46]

Patchset #3 (id:40001) has been deleted

[rmcilroy, 2015-03-18 22:08:36]

rmcilroy@chromium.org changed reviewers:
+ jamescook@chromium.org, jochen@chromium.org

[rmcilroy, 2015-03-18 22:08:36]

jochen@chromium.org: Please review changes in content and gin.
jamescook@chromium.org: Please review changes in ash and extensions.

Thanks!

[James Cook, 2015-03-18 23:20:06]

Generally this seems OK, but I would rewrite the CL description (especially the
first line) to say you're refactoring things to share more code. It's a bit
misleading because the code in ash/shell and extensions/shell doesn't actually
run on in production on either Chrome OS or Chromecast -- it's all test and
development stuff.

https://codereview.chromium.org/1019483002/diff/60001/extensions/shell/browse...
File extensions/shell/browser/shell_content_browser_client.cc (right):

https://codereview.chromium.org/1019483002/diff/60001/extensions/shell/browse...
extensions/shell/browser/shell_content_browser_client.cc:61:
browser_main_parts_(NULL),
Can you switch this to nullptr since you're here?

https://codereview.chromium.org/1019483002/diff/60001/gin/isolate_holder.cc
File gin/isolate_holder.cc (right):

https://codereview.chromium.org/1019483002/diff/60001/gin/isolate_holder.cc#n...
gin/isolate_holder.cc:199: bool IsolateHolder::MapV8FilesForChildProcesses(int*
natives_fd_out,
This function always returns true. Does it need a return value? Should it return
false if one of the DCHECK conditions fails?

https://codereview.chromium.org/1019483002/diff/60001/gin/public/isolate_hold...
File gin/public/isolate_holder.h (right):

https://codereview.chromium.org/1019483002/diff/60001/gin/public/isolate_hold...
gin/public/isolate_holder.h:66: static bool MapV8FilesForChildProcesses(int*
natives_fd_out,
nit: Document what this function does.

[rmcilroy, 2015-03-19 14:41:40]

Thanks for the review, PTAL.

> Generally this seems OK, but I would rewrite the CL description (especially
the
> first line) to say you're refactoring things to share more code. It's a bit
> misleading because the code in ash/shell and extensions/shell doesn't actually
> run on in production on either Chrome OS or Chromecast -- it's all test and
> development stuff.

Done.

https://codereview.chromium.org/1019483002/diff/60001/extensions/shell/browse...
File extensions/shell/browser/shell_content_browser_client.cc (right):

https://codereview.chromium.org/1019483002/diff/60001/extensions/shell/browse...
extensions/shell/browser/shell_content_browser_client.cc:61:
browser_main_parts_(NULL),
On 2015/03/18 23:20:06, James Cook wrote:
> Can you switch this to nullptr since you're here?

Done.

https://codereview.chromium.org/1019483002/diff/60001/gin/isolate_holder.cc
File gin/isolate_holder.cc (right):

https://codereview.chromium.org/1019483002/diff/60001/gin/isolate_holder.cc#n...
gin/isolate_holder.cc:199: bool IsolateHolder::MapV8FilesForChildProcesses(int*
natives_fd_out,
On 2015/03/18 23:20:06, James Cook wrote:
> This function always returns true. Does it need a return value? Should it
return
> false if one of the DCHECK conditions fails?

Yes, this is probably better. Also changed the if's to DCHECKS in the
ContentBrowerClients since we want to catch this failing early (rather than
after the child has already started).

https://codereview.chromium.org/1019483002/diff/60001/gin/public/isolate_hold...
File gin/public/isolate_holder.h (right):

https://codereview.chromium.org/1019483002/diff/60001/gin/public/isolate_hold...
gin/public/isolate_holder.h:66: static bool MapV8FilesForChildProcesses(int*
natives_fd_out,
On 2015/03/18 23:20:06, James Cook wrote:
> nit: Document what this function does.

Done - also renamed to be more precise.

[James Cook, 2015-03-19 15:48:37]

ash and extensions LGTM

https://codereview.chromium.org/1019483002/diff/80001/gin/public/isolate_hold...
File gin/public/isolate_holder.h (right):

https://codereview.chromium.org/1019483002/diff/80001/gin/public/isolate_hold...
gin/public/isolate_holder.h:63: // success.
Thanks for documenting the other methods!

[rmcilroy, 2015-03-20 11:07:24]

On 2015/03/19 15:48:37, James Cook wrote:
> ash and extensions LGTM
> 
>
https://codereview.chromium.org/1019483002/diff/80001/gin/public/isolate_hold...
> File gin/public/isolate_holder.h (right):
> 
>
https://codereview.chromium.org/1019483002/diff/80001/gin/public/isolate_hold...
> gin/public/isolate_holder.h:63: // success.
> Thanks for documenting the other methods!

Thanks James!

Jochen: do the gin and content changes look OK to you?

[jochen (gone - plz use gerrit), 2015-03-20 14:49:48]

lgtm

[rmcilroy, 2015-03-20 14:56:48]

The CQ bit was checked by rmcilroy@chromium.org

[commit-bot: I haz the power, 2015-03-20 14:57:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1019483002/80001

[commit-bot: I haz the power, 2015-03-20 15:26:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-20 15:26:13]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[rmcilroy, 2015-03-20 17:58:09]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-03-20 17:58:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1019483002/#ps100001
(title: "Rebased")

[commit-bot: I haz the power, 2015-03-20 17:58:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1019483002/100001

[commit-bot: I haz the power, 2015-03-20 18:42:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-20 18:42:06]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[rmcilroy, 2015-03-20 22:34:53]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-03-20 22:34:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1019483002/#ps120001
(title: "Fix Mac and Win builds")

[commit-bot: I haz the power, 2015-03-20 22:35:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1019483002/120001

[commit-bot: I haz the power, 2015-03-20 23:44:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-20 23:44:17]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[rmcilroy, 2015-04-06 15:59:00]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-04-06 15:59:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1019483002/#ps160001
(title: "Fix typo")

[commit-bot: I haz the power, 2015-04-06 15:59:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1019483002/160001

[commit-bot: I haz the power, 2015-04-06 16:30:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-06 16:30:12]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[rmcilroy, 2015-04-06 19:55:03]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-04-06 19:55:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1019483002/#ps180001
(title: "Fix Mac and Win")

[commit-bot: I haz the power, 2015-04-06 19:55:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1019483002/180001

[rmcilroy, 2015-04-06 19:56:59]

The CQ bit was unchecked by rmcilroy@chromium.org

[rmcilroy, 2015-04-06 19:58:45]

Patchset #9 (id:180001) has been deleted

[rmcilroy, 2015-04-06 19:58:52]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-04-06 19:58:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1019483002/#ps200001
(title: "Fix Mac and Win")

[commit-bot: I haz the power, 2015-04-06 19:59:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1019483002/200001

[rmcilroy, 2015-04-06 20:17:27]

The CQ bit was unchecked by rmcilroy@chromium.org

[rmcilroy, 2015-04-06 20:27:49]

Patchset #9 (id:200001) has been deleted

[rmcilroy, 2015-04-06 20:27:54]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-04-06 20:27:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1019483002/#ps220001
(title: "Fix Mac and Win")

[commit-bot: I haz the power, 2015-04-06 20:28:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1019483002/220001

[commit-bot: I haz the power, 2015-04-06 21:15:14]

Committed patchset #9 (id:220001)

[commit-bot: I haz the power, 2015-04-06 21:16:20]

Patchset 9 (id:??) landed as
https://crrev.com/54fab5e3b8f16436d3329ecbdb6c40e050278a51
Cr-Commit-Position: refs/heads/master@{#323953}