Don't ever create a file in about:blank when launching packaged apps.

Don't ever create a file in about:blank when launching packaged apps.

Browser tests append about:blank to the command line, which if
passed to a packaged app with write access will result in a file named
'about' being created in the current directory.

This change prevents about:blank being passed to packaged apps as a
launch parameter.

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

[benwells, 2014-05-13 05:45:11]

... since you reviewed my last change in here ...

[tapted, 2014-05-13 06:47:04]

Some suggestions, but lgtm as is too

https://codereview.chromium.org/280703003/diff/1/apps/launcher.cc
File apps/launcher.cc (right):

https://codereview.chromium.org/280703003/diff/1/apps/launcher.cc#newcode356
apps/launcher.cc:356: base::CommandLine::StringType about_blank_url(
if you don't want the #ifdef, you could use

  base::CommandLine::StringType about_blank_url(
      base::FilePath().AppendASCII(content::kAboutBlankURL).value());


.. I think. But the ifdef is fine, since that's a little weird.

https://codereview.chromium.org/280703003/diff/1/apps/launcher.cc#newcode366
apps/launcher.cc:366: command_line.GetArgs()[0] == about_blank_url) {
Seems unfair to disallow any packaged app from opening a file called
'about:blank'.. You could also check that
command_line.HasSwitch(switches::kTestType)

command_line.GetArgs() is also a little heavyweight, and it's used again below -
maybe cache it?

i.e.

  CommandLine::StringVector args = command_line.GetArgs();
  if (args.empty() || (command_line.HasSwitch(switches::kTestType) &&
                       args[0] == about_blank_url)) {
    ..
  }

[benwells, 2014-05-13 11:41:54]

https://codereview.chromium.org/280703003/diff/1/apps/launcher.cc
File apps/launcher.cc (right):

https://codereview.chromium.org/280703003/diff/1/apps/launcher.cc#newcode366
apps/launcher.cc:366: command_line.GetArgs()[0] == about_blank_url) {
On 2014/05/13 06:47:04, tapted wrote:
> Seems unfair to disallow any packaged app from opening a file called
> 'about:blank'.. You could also check that
> command_line.HasSwitch(switches::kTestType)
> 
> command_line.GetArgs() is also a little heavyweight, and it's used again below
-
> maybe cache it?
> 
> i.e.
> 
>   CommandLine::StringVector args = command_line.GetArgs();
>   if (args.empty() || (command_line.HasSwitch(switches::kTestType) &&
>                        args[0] == about_blank_url)) {
>     ..
>   }
> 
> 

Done.

[benwells, 2014-05-13 11:42:26]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2014-05-13 11:43:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/benwells@chromium.org/280703003/20001

[commit-bot: I haz the power, 2014-05-13 15:36:19]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-13 15:45:22]

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

[commit-bot: I haz the power, 2014-05-13 15:45:22]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)

[benwells, 2014-05-13 22:59:51]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2014-05-13 23:06:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/benwells@chromium.org/280703003/40001

[commit-bot: I haz the power, 2014-05-14 02:14:00]

Change committed as 270302