Mac: remove some now-dead code and add a comment.

Mac: remove some now-dead code and add a comment.

I slightly jumped the gun on committing CL 328010, and this patch does some things requested in a review.

BUG=23825
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=29979

I entered the wrong bug number above. It should have been:
BUG=25603

[viettrungluu, 2009-10-23 23:47:59]

Sorry, I had committed that patch. Now I've removed the dead Safari home page
import code and added a comment referencing the bug, as requested. Thanks.

[jeremy, 2009-10-24 00:03:02]

Thanks for the quick turaround!

There's one more vestige of this in the code :)

http://codereview.chromium.org/326024/diff/1/3
File chrome/browser/importer/safari_importer.mm (right):

http://codereview.chromium.org/326024/diff/1/3#newcode66
Line 66: *services_supported |= HISTORY;
There's one more place to clean up, if memory serves the unit test that invokes
this makes assumptions about HOME_PAGE being set, removing those assumptions now
that this is never set would make the test simpler.

[viettrungluu, 2009-10-24 00:12:14]

Thanks to you also. :)

You do indeed remember correctly, and I've made the simplifications. Thanks
again.

On 2009/10/24 00:03:02, jeremy wrote:
> Thanks for the quick turaround!
> 
> There's one more vestige of this in the code :)
> 
> http://codereview.chromium.org/326024/diff/1/3
> File chrome/browser/importer/safari_importer.mm (right):
> 
> http://codereview.chromium.org/326024/diff/1/3#newcode66
> Line 66: *services_supported |= HISTORY;
> There's one more place to clean up, if memory serves the unit test that
invokes
> this makes assumptions about HOME_PAGE being set, removing those assumptions
now
> that this is never set would make the test simpler.

[jeremy, 2009-10-24 00:24:20]

LGTM with following 2 changes.

http://codereview.chromium.org/326024/diff/3001/4003
File chrome/browser/importer/safari_importer_unittest.mm (right):

http://codereview.chromium.org/326024/diff/3001/4003#newcode150
Line 150: EXPECT_EQ(items, HISTORY | FAVORITES);
Could you add back the rest of the ", NONE" checks and add a new one that tests
that HOME_PAGE isn't set.  I know that's redundant but it'll make things a
little more explicit.

http://codereview.chromium.org/326024/diff/3001/4003#newcode161
Line 161: SafariImporter::CanImport(fake_library_dir, &items);
This can now be wrapped in an EXPECT_FALSE() and the comment above can be
removed since that's referring to HOME_PAGE.  With your change, I think
CanImport should always return false.

[viettrungluu, 2009-10-24 00:32:13]

http://codereview.chromium.org/326024/diff/3001/4003
File chrome/browser/importer/safari_importer_unittest.mm (right):

http://codereview.chromium.org/326024/diff/3001/4003#newcode150
Line 150: EXPECT_EQ(items, HISTORY | FAVORITES);
On 2009/10/24 00:24:20, jeremy wrote:
> Could you add back the rest of the ", NONE" checks and add a new one that
tests
> that HOME_PAGE isn't set.  I know that's redundant but it'll make things a
> little more explicit.

Done.

http://codereview.chromium.org/326024/diff/3001/4003#newcode161
Line 161: SafariImporter::CanImport(fake_library_dir, &items);
On 2009/10/24 00:24:20, jeremy wrote:
> This can now be wrapped in an EXPECT_FALSE() and the comment above can be
> removed since that's referring to HOME_PAGE.  With your change, I think
> CanImport should always return false.

Done.