Move the parsing of app.launch related keys out of Extension class.

Move the parsing of app.launch related keys out of Extension class.

BUG=159265
TBR=ben@chromium.org

[Yoyo Zhou, 2013-02-27 02:43:33]

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/browser/ui/tabs...
File chrome/browser/ui/tabs/tab_strip_model_unittest.cc (right):

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/browser/ui/tabs...
chrome/browser/ui/tabs/tab_strip_model_unittest.cc:126: }
Please also add a TearDown() to call
ManifestHandler::ClearRegistryForTesting();

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/common/extensio...
File chrome/common/extensions/app_launch_manifest_handler.cc (right):

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/common/extensio...
chrome/common/extensions/app_launch_manifest_handler.cc:22: namespace errors =
extension_manifest_errors;
nit: I think it's more common for these to be declared above
namespace extensions {
but I don't think it makes a difference here.

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/common/extensio...
chrome/common/extensions/app_launch_manifest_handler.cc:185:
extension->AddWebExtentPattern(pattern);
Web extents should also be ManifestData. You can add that as a TODO someplace,
or do it in this CL. They don't need a specific parser, just an interface like

WebExtentInfo::AddPattern(extension, pattern);

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/common/extensio...
chrome/common/extensions/app_launch_manifest_handler.cc:312: bool
AppLaunchManifestHandler::AlwaysParseForType(Manifest::Type type) {
I'm not sure this is necessary.

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/common/extensio...
chrome/common/extensions/app_launch_manifest_handler.cc:324: keys::kLaunchWidth
kWebURLs should probably be added to this list.

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/common/extensio...
File chrome/common/extensions/app_launch_manifest_handler.h (right):

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/common/extensio...
chrome/common/extensions/app_launch_manifest_handler.h:5: #ifndef
CHROME_COMMON_EXTENSIONS_APP_LAUNCH_MANIFEST_HANDLER_H_
I think I'm liking app_launcher_info.h for the name of this file since most of
the users care about AppLaunchInfo rather than AppLaunchManifestHandler.

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/common/extensio...
chrome/common/extensions/app_launch_manifest_handler.h:20: static const
std::string& GetLaunchWebUrl(const Extension* extension);
Please change the naming so that these all use "URL" rather than "Url".

https://chromiumcodereview.appspot.com/12316077/diff/1/chrome/common/extensio...
chrome/common/extensions/app_launch_manifest_handler.h:38: // absolute. If
relative, it is relative to web_origin.
This comment may need updating. (Where is web_origin?)

[Devlin, 2013-02-27 19:12:47]

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
File chrome/common/extensions/app_launch_manifest_handler.cc (right):

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.cc:116: // launch URL can
be either local (to chrome-extension:// root) or an absolute
nit: capitalize 'l' at the beginning of the sentence.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.cc:275: GURL
new_url(override_url);
It strikes me that if want a copy of override url here, you could just
non-const& the argument to the function.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
File chrome/common/extensions/app_launch_manifest_handler.h (right):

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.h:9: #include
"chrome/common/extensions/manifest_handler.h"
Include what you use: LaunchContainer, GURL.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/mani...
File
chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc
(right):

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/mani...
chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc:14:
virtual void SetUp() OVERRIDE {
while a private: level is default with classes, we still usually declare a
level.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/mani...
chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc:20:
for (size_t i = 0; i < app_launch_keys.size(); ++i)
Just out of curiosity, is there a reason we don't allow the handlers themselves
to register, when they have multiple keys like this?  e.g.

class AppLaunchManifestHandler {
 public:
  static RegisterHandlerForKeys();
};

AppLaunchManifestHandler::RegisterHandlerForKeys() {
  static const char* keys[] = {'key1', 'key2', 'key3'};
  linked_ptr<AppLaunchManifestHandler> handler(
      new AppLaunchManifestHandler);
  for (size_t i = 0; i < arraysize(keys); ++i)
    extensions::ManifestHandler::Register(keys[i], handler);
}

class ValidAppManifestTest : public ExtensionManifestTest {
 protected:
  virtual void SetUp() OVERRIDE {
    ExtensionManifestTest::SetUp();
    extensions::AppLaunchManifestHandler::RegisterHandlerForKeys();
  }
};

It seems a bit cleaner to me, and reduces a lot of the duplicate code in these
tests' SetUp() functions.

Yoyo, thoughts?

[Yoyo Zhou, 2013-02-27 19:40:31]

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/mani...
File
chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc
(right):

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/mani...
chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc:20:
for (size_t i = 0; i < app_launch_keys.size(); ++i)
On 2013/02/27 19:12:48, D Cronin wrote:
> Just out of curiosity, is there a reason we don't allow the handlers
themselves
> to register, when they have multiple keys like this?  e.g.
> 
> class AppLaunchManifestHandler {
>  public:
>   static RegisterHandlerForKeys();
> };
> 
> AppLaunchManifestHandler::RegisterHandlerForKeys() {
>   static const char* keys[] = {'key1', 'key2', 'key3'};
>   linked_ptr<AppLaunchManifestHandler> handler(
>       new AppLaunchManifestHandler);
>   for (size_t i = 0; i < arraysize(keys); ++i)
>     extensions::ManifestHandler::Register(keys[i], handler);
> }
> 
> class ValidAppManifestTest : public ExtensionManifestTest {
>  protected:
>   virtual void SetUp() OVERRIDE {
>     ExtensionManifestTest::SetUp();
>     extensions::AppLaunchManifestHandler::RegisterHandlerForKeys();
>   }
> };
> 
> It seems a bit cleaner to me, and reduces a lot of the duplicate code in these
> tests' SetUp() functions.
> 
> Yoyo, thoughts?

It's in the works and should land today:
https://chromiumcodereview.appspot.com/12253022/

=)

[Joe Thomas, 2013-03-01 23:26:56]

Thanks Yoyo & Devlin for the review. Patch update. please take another look.

https://codereview.chromium.org/12316077/diff/1/chrome/browser/ui/tabs/tab_st...
File chrome/browser/ui/tabs/tab_strip_model_unittest.cc (right):

https://codereview.chromium.org/12316077/diff/1/chrome/browser/ui/tabs/tab_st...
chrome/browser/ui/tabs/tab_strip_model_unittest.cc:126: }
On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> Please also add a TearDown() to call
> ManifestHandler::ClearRegistryForTesting();

Done.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
File chrome/common/extensions/app_launch_manifest_handler.cc (right):

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.cc:22: namespace errors =
extension_manifest_errors;
On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> nit: I think it's more common for these to be declared above
> namespace extensions {
> but I don't think it makes a difference here.

Done.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.cc:116: // launch URL can
be either local (to chrome-extension:// root) or an absolute
On 2013/02/27 19:12:48, D Cronin wrote:
> nit: capitalize 'l' at the beginning of the sentence.

Done.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.cc:185:
extension->AddWebExtentPattern(pattern);
On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> Web extents should also be ManifestData. You can add that as a TODO someplace,
> or do it in this CL. They don't need a specific parser, just an interface like
> 
> WebExtentInfo::AddPattern(extension, pattern);

I will do it in a separate CL. Added a TODO in extension.h where this function
is declared.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.cc:275: GURL
new_url(override_url);
On 2013/02/27 19:12:48, D Cronin wrote:
> It strikes me that if want a copy of override url here, you could just
> non-const& the argument to the function.

Done.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.cc:312: bool
AppLaunchManifestHandler::AlwaysParseForType(Manifest::Type type) {
On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> I'm not sure this is necessary.

This is necessary for TYPE_LEGACY_PACKAGED_APP. While we decide the extension
type in Manifest class c'tor, if kApp key is present but no kLaunchLocalPath, we
consider it as TYPE_LEGACY_PACKAGED_APP type. We need to abort the parsing in
that case (see line 167 in chrome/common/extensions/app_launcher_info.cc in
patch-set 2).

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.cc:324: keys::kLaunchWidth
On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> kWebURLs should probably be added to this list.

I will do it in a separate CL.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
File chrome/common/extensions/app_launch_manifest_handler.h (right):

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.h:5: #ifndef
CHROME_COMMON_EXTENSIONS_APP_LAUNCH_MANIFEST_HANDLER_H_
On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> I think I'm liking app_launcher_info.h for the name of this file since most of
> the users care about AppLaunchInfo rather than AppLaunchManifestHandler.

Yes. That is a better name. Done.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.h:9: #include
"chrome/common/extensions/manifest_handler.h"
On 2013/02/27 19:12:48, D Cronin wrote:
> Include what you use: LaunchContainer, GURL.

Done.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.h:20: static const
std::string& GetLaunchWebUrl(const Extension* extension);
On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> Please change the naming so that these all use "URL" rather than "Url".

Done.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.h:38: // absolute. If
relative, it is relative to web_origin.
On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> This comment may need updating. (Where is web_origin?)

Done. Removed that part.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/mani...
File
chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc
(right):

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/mani...
chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc:14:
virtual void SetUp() OVERRIDE {
On 2013/02/27 19:12:48, D Cronin wrote:
> while a private: level is default with classes, we still usually declare a
> level.

Done.

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
File chrome/browser/extensions/convert_web_app_unittest.cc (right):

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/convert_web_app_unittest.cc:21: #include
"chrome/common/extensions/extension.h"
I will include chrome/common/extensions/manifest_handler.h here.

[Yoyo Zhou, 2013-03-05 20:17:10]

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
File chrome/common/extensions/app_launch_manifest_handler.cc (right):

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.cc:312: bool
AppLaunchManifestHandler::AlwaysParseForType(Manifest::Type type) {
On 2013/03/01 23:26:57, Joe Thomas wrote:
> On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> > I'm not sure this is necessary.
> 
> This is necessary for TYPE_LEGACY_PACKAGED_APP. While we decide the extension
> type in Manifest class c'tor, if kApp key is present but no kLaunchLocalPath,
we
> consider it as TYPE_LEGACY_PACKAGED_APP type. We need to abort the parsing in
> that case (see line 167 in chrome/common/extensions/app_launcher_info.cc in
> patch-set 2).

I see. Slightly wacky but ok.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.cc:324: keys::kLaunchWidth
On 2013/03/01 23:26:57, Joe Thomas wrote:
> On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> > kWebURLs should probably be added to this list.
> 
> I will do it in a separate CL.

Ah, never mind. It should be associated with extents instead.

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
File chrome/common/extensions/app_launch_manifest_handler.h (right):

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.h:38: // absolute. If
relative, it is relative to web_origin.
On 2013/03/01 23:26:57, Joe Thomas wrote:
> On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> > This comment may need updating. (Where is web_origin?)
> 
> Done. Removed that part.

According to the comment in the cc, anyway, it must be absolute. It might be
nice to mention that in the comment.

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/m...
File
chrome/common/extensions/manifest_tests/extension_manifests_launch_unittest.cc
(right):

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/m...
chrome/common/extensions/manifest_tests/extension_manifests_launch_unittest.cc:20:
private:
Actually, these should be protected scope.

[Devlin, 2013-03-08 17:44:21]

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
File chrome/browser/extensions/app_launch_manifest_parser.cc (right):

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/app_launch_manifest_parser.cc:8: #include
"chrome/browser/profiles/profile.h"
nit: Don't need this one either, I think.

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/app_launch_manifest_parser.cc:25:
AppLaunchManifestParser::GetFactoryInstance() {
nit: Not sure why, but style for wrapping functions where we break after the
return type is to put them at the same indentation; i.e.

ProfileKeyedAPIFactory<AppLaunchManifestParser>*
AppLaunchManifestParser::GetFactoryInstance() {

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
File chrome/browser/extensions/app_launch_manifest_parser.h (right):

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/app_launch_manifest_parser.h:8: #include
"base/basictypes.h"
nit: Why do we need this include?

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/a...
File chrome/common/extensions/app_launcher_info.cc (right):

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/a...
chrome/common/extensions/app_launcher_info.cc:99: AppLauncherInfo info =
GetAppLauncherInfo(extension);
If this isn't const &, it'll construct a new AppLauncherInfo here.

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/a...
File chrome/common/extensions/app_launcher_info.h (right):

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/a...
chrome/common/extensions/app_launcher_info.h:57: };
DISALLOW_COPY_AND_ASSIGN

[Joe Thomas, 2013-03-09 01:59:52]

Patch rebased and updated. PTAL

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
File chrome/common/extensions/app_launch_manifest_handler.h (right):

https://codereview.chromium.org/12316077/diff/1/chrome/common/extensions/app_...
chrome/common/extensions/app_launch_manifest_handler.h:38: // absolute. If
relative, it is relative to web_origin.
On 2013/03/05 20:17:10, Yoyo Zhou wrote:
> On 2013/03/01 23:26:57, Joe Thomas wrote:
> > On 2013/02/27 02:43:33, Yoyo Zhou wrote:
> > > This comment may need updating. (Where is web_origin?)
> > 
> > Done. Removed that part.
> 
> According to the comment in the cc, anyway, it must be absolute. It might be
> nice to mention that in the comment.

Done.

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
File chrome/browser/extensions/app_launch_manifest_parser.cc (right):

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/app_launch_manifest_parser.cc:8: #include
"chrome/browser/profiles/profile.h"
On 2013/03/08 17:44:21, D Cronin wrote:
> nit: Don't need this one either, I think.

Done.

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/app_launch_manifest_parser.cc:25:
AppLaunchManifestParser::GetFactoryInstance() {
On 2013/03/08 17:44:21, D Cronin wrote:
> nit: Not sure why, but style for wrapping functions where we break after the
> return type is to put them at the same indentation; i.e.
> 
> ProfileKeyedAPIFactory<AppLaunchManifestParser>*
> AppLaunchManifestParser::GetFactoryInstance() {

Done.

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
File chrome/browser/extensions/app_launch_manifest_parser.h (right):

https://codereview.chromium.org/12316077/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/app_launch_manifest_parser.h:8: #include
"base/basictypes.h"
On 2013/03/08 17:44:21, D Cronin wrote:
> nit: Why do we need this include?

Removed

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/a...
File chrome/common/extensions/app_launcher_info.cc (right):

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/a...
chrome/common/extensions/app_launcher_info.cc:99: AppLauncherInfo info =
GetAppLauncherInfo(extension);
On 2013/03/08 17:44:21, D Cronin wrote:
> If this isn't const &, it'll construct a new AppLauncherInfo here.

Done.

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/a...
File chrome/common/extensions/app_launcher_info.h (right):

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/a...
chrome/common/extensions/app_launcher_info.h:57: };
On 2013/03/08 17:44:21, D Cronin wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/m...
File
chrome/common/extensions/manifest_tests/extension_manifests_launch_unittest.cc
(right):

https://codereview.chromium.org/12316077/diff/8001/chrome/common/extensions/m...
chrome/common/extensions/manifest_tests/extension_manifests_launch_unittest.cc:20:
private:
On 2013/03/05 20:17:11, Yoyo Zhou wrote:
> Actually, these should be protected scope.

Done.

[Yoyo Zhou, 2013-03-11 23:01:37]

LGTM

[Devlin, 2013-03-13 16:36:00]

lgtm

[Joe Thomas, 2013-03-14 00:14:00]

TBR=Ben as the refactoring in components other than extension/ are trivial.

[commit-bot: I haz the power, 2013-03-14 00:15:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/MHX348@motorola.com/12316077/22001

[commit-bot: I haz the power, 2013-03-14 00:15:23]

Failed to apply patch for chrome/browser/ui/extensions/application_launch.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/extensions/application_launch.cc
  Hunk #3 FAILED at 125.
  1 out of 3 hunks FAILED -- saving rejects to file
chrome/browser/ui/extensions/application_launch.cc.rej

Patch:       chrome/browser/ui/extensions/application_launch.cc
Index: chrome/browser/ui/extensions/application_launch.cc
diff --git a/chrome/browser/ui/extensions/application_launch.cc
b/chrome/browser/ui/extensions/application_launch.cc
index
af156362551e7cf972de5d696f4237da48a639d7..a87247313dacfac23832bfc44fd6f621e400d1cf
100644
--- a/chrome/browser/ui/extensions/application_launch.cc
+++ b/chrome/browser/ui/extensions/application_launch.cc
@@ -24,6 +24,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/app_launcher_info.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/manifest_url_handler.h"
@@ -63,7 +64,7 @@ GURL UrlForExtension(const Extension* extension,
            override_url.GetOrigin() == extension->url());
     url = override_url;
   } else {
-    url = extension->GetFullLaunchURL();
+    url = extensions::AppLauncherInfo::GetFullLaunchURL(extension);
   }
 
   // For extensions lacking launch urls, determine a reasonable fallback.
@@ -124,8 +125,10 @@ WebContents* OpenApplicationWindow(
 
   gfx::Rect window_bounds;
   if (extension) {
-    window_bounds.set_width(extension->launch_width());
-    window_bounds.set_height(extension->launch_height());
+    window_bounds.set_width(
+        extensions::AppLauncherInfo::GetLaunchWidth(extension));
+    window_bounds.set_height(
+        extensions::AppLauncherInfo::GetLaunchHeight(extension));
   } else if (!override_bounds.IsEmpty()) {
     window_bounds = override_bounds;
   }

[commit-bot: I haz the power, 2013-03-23 16:18:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/MHX348@motorola.com/12316077/56001

[commit-bot: I haz the power, 2013-03-23 18:24:03]

Retried try job too often on win_rel for step(s) net_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[JoeThomas, 2013-04-13 04:48:36]

Moving this CL to https://chromiumcodereview.appspot.com/14241002/ as there is a
change of e-mail address.

Thanks.