Move most of TestNaClBrowserDelegate to another file.

Move most of TestNaClBrowserDelegate to another file.

It was originally in nacl_file_host_unittest.
Move it to components/ and consider it the "base" test
version. In nacl_file_host_unittest, provide slimmer version
that overrides just what we need for the unittest.

This means that we can reuse the TestNaClBrowserDelegate for
other tests. E.g., I'll be adding one for checking that
the "abi-version" matters for translation caching, and
to test that we need to do a similar trick of having a 
Get/SetPnaclDirectory pair, so as not to modify the PNaCl
files inline.

It also means that we don't need to modify the test file
every time something is added to the NaClBrowserDelegate.


BUG=none
NOTRY=true
(already tried)

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

[Mark Seaborn, 2013-10-31 02:36:37]

LGTM, though I'm not sure what the motivation is.  Maybe you could say a bit
more in the commit message?

https://codereview.chromium.org/51323004/diff/190001/chrome/browser/nacl_host...
File chrome/browser/nacl_host/nacl_file_host_unittest.cc (right):

https://codereview.chromium.org/51323004/diff/190001/chrome/browser/nacl_host...
chrome/browser/nacl_host/nacl_file_host_unittest.cc:23: class
FileHostTestNaclBrowserDelegate : public TestNaClBrowserDelegate {
Can you fix the spelling to be "NaCl" not "Nacl" here, since you're changing the
name?

https://codereview.chromium.org/51323004/diff/190001/components/nacl/common/t...
File components/nacl/common/test_nacl_browser_delegate.h (right):

https://codereview.chromium.org/51323004/diff/190001/components/nacl/common/t...
components/nacl/common/test_nacl_browser_delegate.h:10: // This is a base test
implementation of NaClBrowserDelegate, which
Nit: remove comma, otherwise it looks like you're saying NaClBrowserDelegate
does nothing? :-)

https://codereview.chromium.org/51323004/diff/190001/components/nacl/common/t...
components/nacl/common/test_nacl_browser_delegate.h:18: // 
NaClBrowser::SetDelegate(NULL);   // frees the test delegate.
Indent by 1 more space to match the other SetDelegate() call

[jvoung (off chromium), 2013-10-31 16:59:55]

Thanks -- updated the CL description a bit too.

https://codereview.chromium.org/51323004/diff/190001/chrome/browser/nacl_host...
File chrome/browser/nacl_host/nacl_file_host_unittest.cc (right):

https://codereview.chromium.org/51323004/diff/190001/chrome/browser/nacl_host...
chrome/browser/nacl_host/nacl_file_host_unittest.cc:23: class
FileHostTestNaclBrowserDelegate : public TestNaClBrowserDelegate {
On 2013/10/31 02:36:38, Mark Seaborn wrote:
> Can you fix the spelling to be "NaCl" not "Nacl" here, since you're changing
the
> name?

Done.

https://codereview.chromium.org/51323004/diff/190001/components/nacl/common/t...
File components/nacl/common/test_nacl_browser_delegate.h (right):

https://codereview.chromium.org/51323004/diff/190001/components/nacl/common/t...
components/nacl/common/test_nacl_browser_delegate.h:10: // This is a base test
implementation of NaClBrowserDelegate, which
On 2013/10/31 02:36:38, Mark Seaborn wrote:
> Nit: remove comma, otherwise it looks like you're saying NaClBrowserDelegate
> does nothing? :-)

Done.

https://codereview.chromium.org/51323004/diff/190001/components/nacl/common/t...
components/nacl/common/test_nacl_browser_delegate.h:18: // 
NaClBrowser::SetDelegate(NULL);   // frees the test delegate.
On 2013/10/31 02:36:38, Mark Seaborn wrote:
> Indent by 1 more space to match the other SetDelegate() call

Done.

[commit-bot: I haz the power, 2013-10-31 17:19:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/51323004/290001

[commit-bot: I haz the power, 2013-10-31 18:31:56]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[darktears, 2013-11-04 21:01:15]

On 2013/10/31 18:31:56, I haz the power (commit-bot) wrote:
> Sorry for I got bad news for ya.
> Compile failed with a clobber build on mac.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...
> Your code is likely broken or HEAD is junk. Please ensure your
> code is not broken then alert the build sheriffs.
> Look at the try server FAQ for more details.

Btw I'm also going to move more files from chrome/browser/nacl_host/ and it's
most likely that nacl_file_host_unittest.cc is going to move too.

[commit-bot: I haz the power, 2013-11-06 18:01:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/51323004/490002

[commit-bot: I haz the power, 2013-11-06 18:01:41]

Failed to apply patch for components/nacl/common/test_nacl_browser_delegate.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         components/nacl/common/test_nacl_browser_delegate.h
  Copied chrome/browser/nacl_host/nacl_browser_delegate_impl.h ->
components/nacl/common/test_nacl_browser_delegate.h
  patching file components/nacl/common/test_nacl_browser_delegate.h
  Hunk #2 FAILED at 33.
  1 out of 2 hunks FAILED -- saving rejects to file
components/nacl/common/test_nacl_browser_delegate.h.rej

Patch:   NR 
chrome/browser/nacl_host/nacl_browser_delegate_impl.h->components/nacl/common/test_nacl_browser_delegate.h
Index: components/nacl/common/test_nacl_browser_delegate.h
diff --git a/chrome/browser/nacl_host/nacl_browser_delegate_impl.h
b/components/nacl/common/test_nacl_browser_delegate.h
similarity index 53%
copy from chrome/browser/nacl_host/nacl_browser_delegate_impl.h
copy to components/nacl/common/test_nacl_browser_delegate.h
index
a91a35b44669a37a9efdbfac7b1f1946d864e1ac..2bcba3ea05451699f3702d73655c94eb33d97a0e
100644
--- a/chrome/browser/nacl_host/nacl_browser_delegate_impl.h
+++ b/components/nacl/common/test_nacl_browser_delegate.h
@@ -2,19 +2,28 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_NACL_HOST_NACL_BROWSER_DELEGATE_IMPL_H_
-#define CHROME_BROWSER_NACL_HOST_NACL_BROWSER_DELEGATE_IMPL_H_
+#ifndef COMPONENTS_NACL_COMMON_TEST_NACL_BROWSER_DELEGATE_H_
+#define COMPONENTS_NACL_COMMON_TEST_NACL_BROWSER_DELEGATE_H_
 
+#include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "chrome/browser/extensions/extension_info_map.h"
 #include "components/nacl/common/nacl_browser_delegate.h"
 
-class NaClBrowserDelegateImpl : public NaClBrowserDelegate {
+// This is a base test implementation of NaClBrowserDelegate which
+// does nothing. Individual tests can override the methods further.
+// To use the test delegate:
+//
+//   NaClBrowser::SetDelegate(new RefinedTestNaClBrowserDelegate);
+//
+// and
+//
+//   NaClBrowser::SetDelegate(NULL);   // frees the test delegate.
+class TestNaClBrowserDelegate : public NaClBrowserDelegate {
  public:
-  explicit NaClBrowserDelegateImpl(ExtensionInfoMap* extension_info_map);
-  virtual ~NaClBrowserDelegateImpl();
-
-  virtual void ShowNaClInfobar(int render_process_id, int render_view_id,
+  TestNaClBrowserDelegate();
+  virtual ~TestNaClBrowserDelegate();
+  virtual void ShowNaClInfobar(int render_process_id,
+                               int render_view_id,
                                int error_id) OVERRIDE;
   virtual bool DialogsAreSuppressed() OVERRIDE;
   virtual bool GetCacheDirectory(base::FilePath* cache_dir) OVERRIDE;
@@ -24,20 +33,16 @@ class NaClBrowserDelegateImpl : public NaClBrowserDelegate {
   virtual std::string GetVersionString() const OVERRIDE;
   virtual ppapi::host::HostFactory* CreatePpapiHostFactory(
       content::BrowserPpapiHost* ppapi_host) OVERRIDE;
-  virtual void TryInstallPnacl(
-      const base::Callback<void(bool)>& installed) OVERRIDE;
   virtual bool MapUrlToLocalFilePath(const GURL& url,
-                                     bool is_blocking,
+                                     bool use_blocking_api,
                                      base::FilePath* file_path) OVERRIDE;
+  virtual void TryInstallPnacl(const base::Callback<void(bool)>& installed)
+      OVERRIDE;
   virtual void SetDebugPatterns(std::string debug_patterns) OVERRIDE;
   virtual bool URLMatchesDebugPatterns(const GURL& manifest_url) OVERRIDE;
 
  private:
-  scoped_refptr<ExtensionInfoMap> extension_info_map_;
-  std::vector<URLPattern> debug_patterns_;
-  bool inverse_debug_patterns_;
-  DISALLOW_COPY_AND_ASSIGN(NaClBrowserDelegateImpl);
+  DISALLOW_COPY_AND_ASSIGN(TestNaClBrowserDelegate);
 };
 
-
-#endif  // CHROME_BROWSER_NACL_HOST_NACL_BROWSER_DELEGATE_IMPL_H_
+#endif  // COMPONENTS_NACL_COMMON_TEST_NACL_BROWSER_DELEGATE_H_

[jvoung (off chromium), 2013-11-06 18:02:27]

On 2013/11/04 21:01:15, darktears wrote:
> On 2013/10/31 18:31:56, I haz the power (commit-bot) wrote:
> > Sorry for I got bad news for ya.
> > Compile failed with a clobber build on mac.
> >
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...
> > Your code is likely broken or HEAD is junk. Please ensure your
> > code is not broken then alert the build sheriffs.
> > Look at the try server FAQ for more details.
> 
> Btw I'm also going to move more files from chrome/browser/nacl_host/ and it's
> most likely that nacl_file_host_unittest.cc is going to move too.

Ah okay, but this move is still okay? If so I'll rebase (had to do a different
change to nacl_file_host_unittest.cc last week)

[darktears, 2013-11-06 18:10:24]

On 2013/11/06 18:02:27, jvoung (cr) wrote:
> On 2013/11/04 21:01:15, darktears wrote:
> > On 2013/10/31 18:31:56, I haz the power (commit-bot) wrote:
> > > Sorry for I got bad news for ya.
> > > Compile failed with a clobber build on mac.
> > >
> >
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...
> > > Your code is likely broken or HEAD is junk. Please ensure your
> > > code is not broken then alert the build sheriffs.
> > > Look at the try server FAQ for more details.
> > 
> > Btw I'm also going to move more files from chrome/browser/nacl_host/ and
it's
> > most likely that nacl_file_host_unittest.cc is going to move too.
> 
> Ah okay, but this move is still okay? If so I'll rebase (had to do a different
> change to nacl_file_host_unittest.cc last week)

No go ahead land this first.

[commit-bot: I haz the power, 2013-11-06 18:32:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/51323004/870001

[commit-bot: I haz the power, 2013-11-06 18:58:22]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-11-06 22:21:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/51323004/870001

[commit-bot: I haz the power, 2013-11-06 22:45:08]

Change committed as 233404