Search for history items that was imported from IE is not working.

chrome/browser/importer/ie_importer_browsertest_win.cc

Index: chrome/browser/importer/ie_importer_browsertest_win.cc
diff --git a/chrome/browser/importer/ie_importer_browsertest_win.cc b/chrome/browser/importer/ie_importer_browsertest_win.cc
index d41961481d398b59e5dc852c5ac4203c92dab4db..a2bb495e68c152bb82449244b9c862ff5f647bd1 100644
--- a/chrome/browser/importer/ie_importer_browsertest_win.cc
+++ b/chrome/browser/importer/ie_importer_browsertest_win.cc
@@ -85,6 +85,10 @@ const base::char16 kIEIdentifyUrl[] =
     L"http://A79029D6-753E-4e27-B807-3D46AB1545DF.com:8080/path?key=value";
 const base::char16 kIEIdentifyTitle[] =
     L"Unittest GUID";
+const base::char16 kIECacheItemUrl[] =
+    L"http://B2EF40C8-2569-4D7E-97EA-BAD9DF468D9C.com";
+const base::char16 kIECacheItemTitle[] =
+    L"Unittest Cache Item GUID";
 
 const base::char16 kFaviconStreamSuffix[] = L"url:favicon:$DATA";
 const char kDummyFaviconImageData[] =
@@ -241,7 +245,7 @@ class TestObserver : public ProfileWriter,
       EXPECT_EQ(arraysize(kIEFaviconGroup), favicon_count_);
     }
     if (importer_items_ & importer::HISTORY)
-      EXPECT_EQ(1, history_count_);
+      EXPECT_EQ(2, history_count_);
     if (importer_items_ & importer::HOME_PAGE)
       EXPECT_EQ(1, homepage_count_);
     if ((importer_items_ & importer::PASSWORDS) && (ie_version_ == IE7))
@@ -274,12 +278,25 @@ class TestObserver : public ProfileWriter,
 
   virtual void AddHistoryPage(const history::URLRows& page,
                               history::VisitSource visit_source) {
+    bool cache_item_found = false;
+    bool history_item_found = false;
     // Importer should read the specified URL.
     for (size_t i = 0; i < page.size(); ++i) {
       if (page[i].title() == kIEIdentifyTitle &&
-          page[i].url() == GURL(kIEIdentifyUrl))
+          page[i].url() == GURL(kIEIdentifyUrl)) {
+        EXPECT_FALSE(page[i].hidden());
+        history_item_found = true;
         ++history_count_;
+      }
+      if (page[i].title() == kIECacheItemTitle &&
+          page[i].url() == GURL(kIECacheItemUrl)) {
+        EXPECT_TRUE(page[i].hidden());
+        cache_item_found = true;
+        ++history_count_;
+      }
     }
+    EXPECT_TRUE(history_item_found);
+    EXPECT_TRUE(cache_item_found);
     EXPECT_EQ(history::SOURCE_IE_IMPORTED, visit_source);
   }
 
@@ -480,7 +497,14 @@ IN_PROC_BROWSER_TEST_F(IEImporterBrowserTest, IEImporter) {
   res = url_history_stg2.CreateInstance(CLSID_CUrlHistory, NULL,
                                         CLSCTX_INPROC_SERVER);
   ASSERT_TRUE(res == S_OK);

[gab, 2014/12/23 22:00:58]

It's kind of overkill to store |res| in a variable only to assert it. And it's
ugly to re-use the same variable to do this over and over again.

Could you please inline the calls inside the asserts and get rid of |res|?

Thanks!

[Alexey Seren, 2014/12/24 07:23:19]

Acknowledged.

-  res = url_history_stg2->AddUrl(kIEIdentifyUrl, kIEIdentifyTitle, 0);
+  // Usage of ADDURL_ADDTOHISTORYANDCACHE and ADDURL_ADDTOCACHE flags
+  // is explained in the article:
+  // http://msdn.microsoft.com/ru-ru/aa767730
+  res = url_history_stg2->AddUrl(kIEIdentifyUrl, kIEIdentifyTitle,
+                                 ADDURL_ADDTOHISTORYANDCACHE);
+  ASSERT_TRUE(res == S_OK);
+  res = url_history_stg2->AddUrl(kIECacheItemUrl, kIECacheItemTitle,
+                                 ADDURL_ADDTOCACHE);
   ASSERT_TRUE(res == S_OK);
 
   // Starts to import the above settings.
@@ -504,6 +528,7 @@ IN_PROC_BROWSER_TEST_F(IEImporterBrowserTest, IEImporter) {
 
   // Cleans up.
   url_history_stg2->DeleteUrl(kIEIdentifyUrl, 0);
+  url_history_stg2->DeleteUrl(kIECacheItemUrl, 0);
   url_history_stg2.Release();
 }