Remove stale code to delete the metro user data dir. It never shipped.

Remove stale code to delete the metro user data dir. It never shipped.

This more or less reverts r149968.

BUG=none
TBR=sky@chromium.org

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

[gab, 2014-04-02 15:16:56]

CC:msw

lgtm w/ nits

I also recall a method like GetAlternateUserDataDir() which would return the
user-data-dir not currently in use; I can't find it anymore though, I guess it
was already removed in a prior CL..?

I think we should hold before landing this for a fix to the bug introduced in
https://codereview.chromium.org/208393020/ as doing so will ease the merge of
that fix to M35.

Thanks!
Gab

https://codereview.chromium.org/218683013/diff/1/chrome/installer/setup/unins...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/218683013/diff/1/chrome/installer/setup/unins...
chrome/installer/setup/uninstall.cc:420: DeleteResult DeleteLocalState(const
base::FilePath& local_state_folder,
Why is this called DeleteLocalState if it deletes the entrie user-data-dir?!

https://codereview.chromium.org/218683013/diff/1/chrome/installer/util/helper.h
File chrome/installer/util/helper.h (right):

https://codereview.chromium.org/218683013/diff/1/chrome/installer/util/helper...
chrome/installer/util/helper.h:28: // inside "Document And Settings\<user>\Local
Settings.". Note that this is the
This is the XP path, the previous comment was more specific, which I prefered.

[grt (UTC plus 2), 2014-04-02 16:53:09]

Thanks. I agree with holding off on landing until the other fix lands. PTAL.

https://codereview.chromium.org/218683013/diff/1/chrome/installer/setup/unins...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/218683013/diff/1/chrome/installer/setup/unins...
chrome/installer/setup/uninstall.cc:420: DeleteResult DeleteLocalState(const
base::FilePath& local_state_folder,
On 2014/04/02 15:16:57, gab wrote:
> Why is this called DeleteLocalState if it deletes the entrie user-data-dir?!

It deletes all locally-stored state (which happens to be in a directory
ordinarily referred to as the user data dir). I guess you're asking me to rename
it since you'd rather it be DeleteUserDataDir since that's closer to how this
state is generally referred to? I will do this since I think it's what you mean.
Let me know if you really meant something else.

https://codereview.chromium.org/218683013/diff/1/chrome/installer/util/helper.h
File chrome/installer/util/helper.h (right):

https://codereview.chromium.org/218683013/diff/1/chrome/installer/util/helper...
chrome/installer/util/helper.h:28: // inside "Document And Settings\<user>\Local
Settings.". Note that this is the
On 2014/04/02 15:16:57, gab wrote:
> This is the XP path, the previous comment was more specific, which I prefered.

Done.

https://codereview.chromium.org/218683013/diff/20001/chrome/installer/setup/u...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/218683013/diff/20001/chrome/installer/setup/u...
chrome/installer/setup/uninstall.cc:54: namespace installer {
In patch set 2, I've moved all private functions into the unnamed namespace.
This required some code motion, which explains the big deleted and added blocks.

[gab, 2014-04-02 17:20:12]

lgtm w/ a single comment nit that I missed in the last review.

Cheers!
Gab

https://codereview.chromium.org/218683013/diff/1/chrome/installer/setup/unins...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/218683013/diff/1/chrome/installer/setup/unins...
chrome/installer/setup/uninstall.cc:420: DeleteResult DeleteLocalState(const
base::FilePath& local_state_folder,
On 2014/04/02 16:53:09, grt wrote:
> On 2014/04/02 15:16:57, gab wrote:
> > Why is this called DeleteLocalState if it deletes the entrie user-data-dir?!
> 
> It deletes all locally-stored state (which happens to be in a directory
> ordinarily referred to as the user data dir). I guess you're asking me to
rename
> it since you'd rather it be DeleteUserDataDir since that's closer to how this
> state is generally referred to? I will do this since I think it's what you
mean.
> Let me know if you really meant something else.

Yes that's what I meant, sorry for being unclear :)!

https://codereview.chromium.org/218683013/diff/20001/chrome/installer/setup/u...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/218683013/diff/20001/chrome/installer/setup/u...
chrome/installer/setup/uninstall.cc:54: namespace installer {
On 2014/04/02 16:53:10, grt wrote:
> In patch set 2, I've moved all private functions into the unnamed namespace.
> This required some code motion, which explains the big deleted and added
blocks.

Sounds good, I assume the code didn't change (didn't review closely).

https://codereview.chromium.org/218683013/diff/20001/chrome/installer/setup/u...
chrome/installer/setup/uninstall.cc:410: // Deletes a product's user data
directory.
I think this method used to take a |product| argument, but it no longer does and
this comment should no longer refer to "product" IMO.

[grt (UTC plus 2), 2014-04-02 18:04:12]

Whaddya think?

https://codereview.chromium.org/218683013/diff/20001/chrome/installer/setup/u...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/218683013/diff/20001/chrome/installer/setup/u...
chrome/installer/setup/uninstall.cc:410: // Deletes a product's user data
directory.
On 2014/04/02 17:20:14, gab wrote:
> I think this method used to take a |product| argument, but it no longer does
and
> this comment should no longer refer to "product" IMO.

Done.

[gab, 2014-04-02 18:55:23]

lgtm++

[grt (UTC plus 2), 2014-04-03 10:47:56]

The CQ bit was checked by grt@chromium.org

[commit-bot: I haz the power, 2014-04-03 10:48:00]

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

[commit-bot: I haz the power, 2014-04-03 11:15:14]

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

[commit-bot: I haz the power, 2014-04-03 11:15:15]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[grt (UTC plus 2), 2014-04-03 13:21:30]

TBR sky for the removal of a stale Win8 Metro constant in
chrome/common/chrome_constants.{cc,h}.

[grt (UTC plus 2), 2014-04-03 13:22:13]

The CQ bit was checked by grt@chromium.org

[commit-bot: I haz the power, 2014-04-03 13:23:06]

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

[commit-bot: I haz the power, 2014-04-03 14:59:19]

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

[commit-bot: I haz the power, 2014-04-03 14:59:19]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[grt (UTC plus 2), 2014-04-03 15:17:00]

The CQ bit was checked by grt@chromium.org

[commit-bot: I haz the power, 2014-04-03 15:17:08]

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

[commit-bot: I haz the power, 2014-04-03 17:10:57]

Change committed as 261457

[gab, 2014-04-04 19:40:26]

I just thought, we can also remove the special EULA code that was added for the
split user-data-dir too in http://crbug.com/131033.

[Nico, 2014-07-21 14:47:44]

https://codereview.chromium.org/218683013/diff/40001/chrome/installer/setup/u...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/218683013/diff/40001/chrome/installer/setup/u...
chrome/installer/setup/uninstall.cc:433: const base::FilePath
user_data_dir(user_data_dir.DirName());
clang remarks:

..\..\chrome\installer\setup\uninstall.cc(445,40) :  warning(clang): variable
'user_data_dir' is uninitialized when used within its own initialization
[-Wuninitialized]
    const base::FilePath user_data_dir(user_data_dir.DirName());
                         ~~~~~~~~~~~~~ ^~~~~~~~~~~~~
1 warning generated.

Maybe you want to give the variable in the inner scope a different name? As is,
it's used for its own initialization. (Since this code doesn't seem to work,
maybe it's unused and can be deleted?)

[Nico, 2014-07-23 00:41:03]

On 2014/07/21 14:47:44, Nico (away) wrote:
>
https://codereview.chromium.org/218683013/diff/40001/chrome/installer/setup/u...
> File chrome/installer/setup/uninstall.cc (right):
> 
>
https://codereview.chromium.org/218683013/diff/40001/chrome/installer/setup/u...
> chrome/installer/setup/uninstall.cc:433: const base::FilePath
> user_data_dir(user_data_dir.DirName());
> clang remarks:
> 
> ..\..\chrome\installer\setup\uninstall.cc(445,40) :  warning(clang): variable
> 'user_data_dir' is uninitialized when used within its own initialization
> [-Wuninitialized]
>     const base::FilePath user_data_dir(user_data_dir.DirName());
>                          ~~~~~~~~~~~~~ ^~~~~~~~~~~~~
> 1 warning generated.
> 
> Maybe you want to give the variable in the inner scope a different name? As
is,
> it's used for its own initialization. (Since this code doesn't seem to work,
> maybe it's unused and can be deleted?)

gab, grt: ping ^

[grt (UTC plus 2), 2014-07-23 01:21:42]

On 2014/07/23 00:41:03, Nico (away) wrote:
> On 2014/07/21 14:47:44, Nico (away) wrote:
> >
>
https://codereview.chromium.org/218683013/diff/40001/chrome/installer/setup/u...
> > File chrome/installer/setup/uninstall.cc (right):
> > 
> >
>
https://codereview.chromium.org/218683013/diff/40001/chrome/installer/setup/u...
> > chrome/installer/setup/uninstall.cc:433: const base::FilePath
> > user_data_dir(user_data_dir.DirName());
> > clang remarks:
> > 
> > ..\..\chrome\installer\setup\uninstall.cc(445,40) :  warning(clang):
variable
> > 'user_data_dir' is uninitialized when used within its own initialization
> > [-Wuninitialized]
> >     const base::FilePath user_data_dir(user_data_dir.DirName());
> >                          ~~~~~~~~~~~~~ ^~~~~~~~~~~~~
> > 1 warning generated.
> > 
> > Maybe you want to give the variable in the inner scope a different name? As
> is,
> > it's used for its own initialization. (Since this code doesn't seem to work,
> > maybe it's unused and can be deleted?)
> 
> gab, grt: ping ^

ack. will investigate. thanks.