Fix crashes loading internal chrome pages powered by Mojo

Fix crashes loading internal chrome pages powered by Mojo

R=rockot@chromium.org

Committed: https://crrev.com/9875db491a116291deaab8309834b0fa8d737dfa
Cr-Commit-Position: refs/heads/master@{#428846}

[Ben Goodger (Google), 2016-10-31 21:33:48]

The CQ bit was checked by ben@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-31 21:34:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Rockot(use gerrit already), 2016-10-31 21:34:49]

lgtm

[Ben Goodger (Google), 2016-10-31 21:35:11]

The CQ bit was checked by ben@chromium.org

[commit-bot: I haz the power, 2016-10-31 21:35:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom Sepez, 2016-10-31 21:45:04]

tsepez@chromium.org changed reviewers:
+ tsepez@chromium.org

[Tom Sepez, 2016-10-31 21:45:05]

RS LGTM if it gets through our parser.

https://codereview.chromium.org/2469533002/diff/1/chrome/browser/chrome_conte...
File chrome/browser/chrome_content_browser_manifest_overlay.json (right):

https://codereview.chromium.org/2469533002/diff/1/chrome/browser/chrome_conte...
chrome/browser/chrome_content_browser_manifest_overlay.json:53: // TODO(beng):
These should be moved to a separate capability.
nit: Strictly speaking, Comments aren't allowed in JSON :(.  See www.json.org.

[Ben Goodger (Google), 2016-10-31 21:52:06]

On 2016/10/31 21:45:05, Tom Sepez wrote:
> RS LGTM if it gets through our parser.
> 
>
https://codereview.chromium.org/2469533002/diff/1/chrome/browser/chrome_conte...
> File chrome/browser/chrome_content_browser_manifest_overlay.json (right):
> 
>
https://codereview.chromium.org/2469533002/diff/1/chrome/browser/chrome_conte...
> chrome/browser/chrome_content_browser_manifest_overlay.json:53: // TODO(beng):
> These should be moved to a separate capability.
> nit: Strictly speaking, Comments aren't allowed in JSON :(.  See
http://www.json.org.

We pass manifests thru the JSON comment eater.

[Will Harris, 2016-10-31 21:54:07]

wfh@chromium.org changed reviewers:
+ wfh@chromium.org

[Will Harris, 2016-10-31 21:54:08]

https://codereview.chromium.org/2469533002/diff/1/chrome/browser/chrome_conte...
File chrome/browser/chrome_content_browser_manifest_overlay.json (right):

https://codereview.chromium.org/2469533002/diff/1/chrome/browser/chrome_conte...
chrome/browser/chrome_content_browser_manifest_overlay.json:54:
"mojom::OmniboxPageHandler",
how do we know that we have all the interfaces listed and none are missing?

[Ken Rockot(use gerrit already), 2016-10-31 21:59:59]

We don't. And without tests for random features, we can't. It's a chicken
and egg problem in this case, and having the filter landed on dev/canary
makes it possible to discover missing interfaces. If there's a more
specific repro (like there is for the Web UI pages) that helps.

On Mon, Oct 31, 2016 at 2:54 PM, <wfh@chromium.org> wrote:

>
> https://codereview.chromium.org/2469533002/diff/1/chrome/
> browser/chrome_content_browser_manifest_overlay.json
> File chrome/browser/chrome_content_browser_manifest_overlay.json
> (right):
>
> https://codereview.chromium.org/2469533002/diff/1/chrome/
> browser/chrome_content_browser_manifest_overlay.json#newcode54
> chrome/browser/chrome_content_browser_manifest_overlay.json:54:
> "mojom::OmniboxPageHandler",
> how do we know that we have all the interfaces listed and none are
> missing?
>
> https://codereview.chromium.org/2469533002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2016-10-31 22:35:38]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-31 22:37:54]

Description was changed from

==========
Fix crashes loading internal chrome pages powered by Mojo

R=rockot@chromium.org
==========

to

==========
Fix crashes loading internal chrome pages powered by Mojo

R=rockot@chromium.org

Committed: https://crrev.com/9875db491a116291deaab8309834b0fa8d737dfa
Cr-Commit-Position: refs/heads/master@{#428846}
==========

[commit-bot: I haz the power, 2016-10-31 22:37:55]

Patchset 1 (id:??) landed as
https://crrev.com/9875db491a116291deaab8309834b0fa8d737dfa
Cr-Commit-Position: refs/heads/master@{#428846}