Avoid blocking the PNaCl translator processes when chrome NaCl GDB flag is on.

chrome/browser/nacl_host/nacl_browser_delegate_impl.cc

Index: chrome/browser/nacl_host/nacl_browser_delegate_impl.cc
diff --git a/chrome/browser/nacl_host/nacl_browser_delegate_impl.cc b/chrome/browser/nacl_host/nacl_browser_delegate_impl.cc
index 9e7b83f7f7c2dd83f12fd358b8c8a048836df68a..b14b686c9d4e539202caee3a6b52083cde4165ca 100644
--- a/chrome/browser/nacl_host/nacl_browser_delegate_impl.cc
+++ b/chrome/browser/nacl_host/nacl_browser_delegate_impl.cc
@@ -164,11 +164,16 @@ void NaClBrowserDelegateImpl::SetDebugPatterns(std::string debug_patterns) {
   for (std::vector<std::string>::iterator iter = patterns.begin();
        iter != patterns.end(); ++iter) {
     URLPattern pattern;
+    // Allow chrome:// schema, which is used to filter out the internal
+    // PNaCl translator. Also allow chrome-extension:// schema (which
+    // can have NaCl modules). The default is to disallow these schema
+    // since they can be dangerous in the context of chrome extension
+    // permissions, but they are okay here, for NaCl GDB avoidance.
+    pattern.SetValidSchemes(URLPattern::SCHEME_ALL);

[teravest, 2014/02/25 18:51:29]

I'm confused. Why do you want to set the scheme here, even when the pattern
isn't parsed successfully?

It seems like the existing code already sets valid schemes properly for
 all parsed patterns. Is the problem that Parse() fails without
SetValidSchemes()?


It's weird to have "pattern.SetValidSchemes(URLPattern::SCHEME_ALL)" called
twice here. I don't understand why that would be necessary.

[jvoung (off chromium), 2014/02/25 23:52:31]

Yes, the problem is that Parse() would otherwise fail when it sees a pattern
like "chrome://*/foo*". It would however be happy with the less strict
"*://*/foo*".
There's two points at which the scheme matter:

(1) When you are setting up the pattern matcher. E.g., when you're trying to
parse *://ssh*, or chrome://*

(2) When you're doing the match against a real URL. E.g.,
chrome-extension://abcdefg/ssh_client.nmf.

The added early setting for SCHEME_ALL helps with case (1).

The existing setting for SCHEME_ALL helps with *:// in case (1) plus
chrome-extensions://abcdefb/ssh_client.nmf in case (2). It did not require
setting SCHEME_ALL prior to Parse() because the pattern had a wild card scheme.

I believe that if we had made the SSH pattern more strict like
"chrome-extension://*/*ssh_client.nmf instead of having a wild card at the
schema position, then the pattern matcher would not have reset to only allowing
http and https.

So, we could change the pattern and get rid of the second SetValidSchemes(). An
argument to keep the second one is if there is a chance that someone specifies a
filter pattern manually on the commandline and puts a *:// even though they knew
it only needed to match chrome-extension://.

As for why set it unconditionally to begin with, even when Parse() returns
false? I don't think that matters much since the pattern won't get added to the
debug_patterns_ vector and will be dropped.

     if (pattern.Parse(*iter) == URLPattern::PARSE_SUCCESS) {
       // If URL pattern has scheme equal to *, Parse method resets valid
       // schemes mask to http and https only, so we need to reset it after
-      // Parse to include chrome-extension scheme that can be used by NaCl
-      // manifest files.
+      // Parse to re-include chrome-extension and chrome schema.
       pattern.SetValidSchemes(URLPattern::SCHEME_ALL);
       debug_patterns_.push_back(pattern);
     }