Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(215)

Side by Side Diff: chrome/browser/safe_browsing/safe_browsing_store_file.cc

Issue 744183002: More explicit thread checking in SafeBrowsingDatabase. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@a3_deadcode
Patch Set: crbug.com/338486 forces thread-checks to be disabled in ~SafeBrowsingStoreFile() Created 6 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/safe_browsing/safe_browsing_store_file.h" 5 #include "chrome/browser/safe_browsing/safe_browsing_store_file.h"
6 6
7 #include "base/files/file_util.h" 7 #include "base/files/file_util.h"
8 #include "base/files/scoped_file.h" 8 #include "base/files/scoped_file.h"
9 #include "base/md5.h" 9 #include "base/md5.h"
10 #include "base/metrics/histogram.h" 10 #include "base/metrics/histogram.h"
(...skipping 540 matching lines...) Expand 10 before | Expand all | Expand 10 after
551 551
552 return static_cast<int64>(ftell(file.get())) == size; 552 return static_cast<int64>(ftell(file.get())) == size;
553 } 553 }
554 554
555 } // namespace 555 } // namespace
556 556
557 SafeBrowsingStoreFile::SafeBrowsingStoreFile() 557 SafeBrowsingStoreFile::SafeBrowsingStoreFile()
558 : chunks_written_(0), empty_(false), corruption_seen_(false) {} 558 : chunks_written_(0), empty_(false), corruption_seen_(false) {}
559 559
560 SafeBrowsingStoreFile::~SafeBrowsingStoreFile() { 560 SafeBrowsingStoreFile::~SafeBrowsingStoreFile() {
561 // Thread-checking is disabled in the destructor due to crbug.com/338486.
562 DetachFromThread();
563
561 Close(); 564 Close();
562 } 565 }
563 566
564 bool SafeBrowsingStoreFile::Delete() { 567 bool SafeBrowsingStoreFile::Delete() {
568 DCHECK(CalledOnValidThread());
569
565 // The database should not be open at this point. But, just in 570 // The database should not be open at this point. But, just in
566 // case, close everything before deleting. 571 // case, close everything before deleting.
567 if (!Close()) { 572 if (!Close()) {
568 NOTREACHED(); 573 NOTREACHED();
569 return false; 574 return false;
570 } 575 }
571 576
572 return DeleteStore(filename_); 577 return DeleteStore(filename_);
573 } 578 }
574 579
575 bool SafeBrowsingStoreFile::CheckValidity() { 580 bool SafeBrowsingStoreFile::CheckValidity() {
581 DCHECK(CalledOnValidThread());
582
576 // The file was either empty or never opened. The empty case is 583 // The file was either empty or never opened. The empty case is
577 // presumed not to be invalid. The never-opened case can happen if 584 // presumed not to be invalid. The never-opened case can happen if
578 // BeginUpdate() fails for any databases, and should already have 585 // BeginUpdate() fails for any databases, and should already have
579 // caused the corruption callback to fire. 586 // caused the corruption callback to fire.
580 if (!file_.get()) 587 if (!file_.get())
581 return true; 588 return true;
582 589
583 if (!FileRewind(file_.get())) 590 if (!FileRewind(file_.get()))
584 return OnCorruptDatabase(); 591 return OnCorruptDatabase();
585 592
(...skipping 27 matching lines...) Expand all
613 if (!ReadAndVerifyChecksum(file_.get(), &context)) { 620 if (!ReadAndVerifyChecksum(file_.get(), &context)) {
614 RecordFormatEvent(FORMAT_EVENT_VALIDITY_CHECKSUM_FAILURE); 621 RecordFormatEvent(FORMAT_EVENT_VALIDITY_CHECKSUM_FAILURE);
615 return OnCorruptDatabase(); 622 return OnCorruptDatabase();
616 } 623 }
617 624
618 return true; 625 return true;
619 } 626 }
620 627
621 void SafeBrowsingStoreFile::Init(const base::FilePath& filename, 628 void SafeBrowsingStoreFile::Init(const base::FilePath& filename,
622 const base::Closure& corruption_callback) { 629 const base::Closure& corruption_callback) {
630 DCHECK(CalledOnValidThread());
623 filename_ = filename; 631 filename_ = filename;
624 corruption_callback_ = corruption_callback; 632 corruption_callback_ = corruption_callback;
625 } 633 }
626 634
627 bool SafeBrowsingStoreFile::BeginChunk() { 635 bool SafeBrowsingStoreFile::BeginChunk() {
636 DCHECK(CalledOnValidThread());
628 return ClearChunkBuffers(); 637 return ClearChunkBuffers();
629 } 638 }
630 639
631 bool SafeBrowsingStoreFile::WriteAddPrefix(int32 chunk_id, SBPrefix prefix) { 640 bool SafeBrowsingStoreFile::WriteAddPrefix(int32 chunk_id, SBPrefix prefix) {
641 DCHECK(CalledOnValidThread());
632 add_prefixes_.push_back(SBAddPrefix(chunk_id, prefix)); 642 add_prefixes_.push_back(SBAddPrefix(chunk_id, prefix));
633 return true; 643 return true;
634 } 644 }
635 645
636 bool SafeBrowsingStoreFile::GetAddPrefixes(SBAddPrefixes* add_prefixes) { 646 bool SafeBrowsingStoreFile::GetAddPrefixes(SBAddPrefixes* add_prefixes) {
647 DCHECK(CalledOnValidThread());
648
637 add_prefixes->clear(); 649 add_prefixes->clear();
638 if (!base::PathExists(filename_)) 650 if (!base::PathExists(filename_))
639 return true; 651 return true;
640 652
641 StateInternal db_state; 653 StateInternal db_state;
642 if (!ReadDbStateHelper(filename_, &db_state)) 654 if (!ReadDbStateHelper(filename_, &db_state))
643 return OnCorruptDatabase(); 655 return OnCorruptDatabase();
644 656
645 add_prefixes->swap(db_state.add_prefixes_); 657 add_prefixes->swap(db_state.add_prefixes_);
646 return true; 658 return true;
647 } 659 }
648 660
649 bool SafeBrowsingStoreFile::GetAddFullHashes( 661 bool SafeBrowsingStoreFile::GetAddFullHashes(
650 std::vector<SBAddFullHash>* add_full_hashes) { 662 std::vector<SBAddFullHash>* add_full_hashes) {
663 DCHECK(CalledOnValidThread());
664
651 add_full_hashes->clear(); 665 add_full_hashes->clear();
652 if (!base::PathExists(filename_)) 666 if (!base::PathExists(filename_))
653 return true; 667 return true;
654 668
655 StateInternal db_state; 669 StateInternal db_state;
656 if (!ReadDbStateHelper(filename_, &db_state)) 670 if (!ReadDbStateHelper(filename_, &db_state))
657 return OnCorruptDatabase(); 671 return OnCorruptDatabase();
658 672
659 add_full_hashes->swap(db_state.add_full_hashes_); 673 add_full_hashes->swap(db_state.add_full_hashes_);
660 return true; 674 return true;
661 } 675 }
662 676
663 bool SafeBrowsingStoreFile::WriteAddHash(int32 chunk_id, 677 bool SafeBrowsingStoreFile::WriteAddHash(int32 chunk_id,
664 const SBFullHash& full_hash) { 678 const SBFullHash& full_hash) {
679 DCHECK(CalledOnValidThread());
665 add_hashes_.push_back(SBAddFullHash(chunk_id, full_hash)); 680 add_hashes_.push_back(SBAddFullHash(chunk_id, full_hash));
666 return true; 681 return true;
667 } 682 }
668 683
669 bool SafeBrowsingStoreFile::WriteSubPrefix(int32 chunk_id, 684 bool SafeBrowsingStoreFile::WriteSubPrefix(int32 chunk_id,
670 int32 add_chunk_id, 685 int32 add_chunk_id,
671 SBPrefix prefix) { 686 SBPrefix prefix) {
687 DCHECK(CalledOnValidThread());
672 sub_prefixes_.push_back(SBSubPrefix(chunk_id, add_chunk_id, prefix)); 688 sub_prefixes_.push_back(SBSubPrefix(chunk_id, add_chunk_id, prefix));
673 return true; 689 return true;
674 } 690 }
675 691
676 bool SafeBrowsingStoreFile::WriteSubHash(int32 chunk_id, int32 add_chunk_id, 692 bool SafeBrowsingStoreFile::WriteSubHash(int32 chunk_id, int32 add_chunk_id,
677 const SBFullHash& full_hash) { 693 const SBFullHash& full_hash) {
694 DCHECK(CalledOnValidThread());
678 sub_hashes_.push_back(SBSubFullHash(chunk_id, add_chunk_id, full_hash)); 695 sub_hashes_.push_back(SBSubFullHash(chunk_id, add_chunk_id, full_hash));
679 return true; 696 return true;
680 } 697 }
681 698
682 bool SafeBrowsingStoreFile::OnCorruptDatabase() { 699 bool SafeBrowsingStoreFile::OnCorruptDatabase() {
700 DCHECK(CalledOnValidThread());
701
683 if (!corruption_seen_) 702 if (!corruption_seen_)
684 RecordFormatEvent(FORMAT_EVENT_FILE_CORRUPT); 703 RecordFormatEvent(FORMAT_EVENT_FILE_CORRUPT);
685 corruption_seen_ = true; 704 corruption_seen_ = true;
686 705
687 corruption_callback_.Run(); 706 corruption_callback_.Run();
688 707
689 // Return false as a convenience to callers. 708 // Return false as a convenience to callers.
690 return false; 709 return false;
691 } 710 }
692 711
693 bool SafeBrowsingStoreFile::Close() { 712 bool SafeBrowsingStoreFile::Close() {
713 DCHECK(CalledOnValidThread());
714
694 ClearUpdateBuffers(); 715 ClearUpdateBuffers();
695 716
696 // Make sure the files are closed. 717 // Make sure the files are closed.
697 file_.reset(); 718 file_.reset();
698 new_file_.reset(); 719 new_file_.reset();
699 return true; 720 return true;
700 } 721 }
701 722
702 bool SafeBrowsingStoreFile::BeginUpdate() { 723 bool SafeBrowsingStoreFile::BeginUpdate() {
724 DCHECK(CalledOnValidThread());
703 DCHECK(!file_.get() && !new_file_.get()); 725 DCHECK(!file_.get() && !new_file_.get());
704 726
705 // Structures should all be clear unless something bad happened. 727 // Structures should all be clear unless something bad happened.
706 DCHECK(add_chunks_cache_.empty()); 728 DCHECK(add_chunks_cache_.empty());
707 DCHECK(sub_chunks_cache_.empty()); 729 DCHECK(sub_chunks_cache_.empty());
708 DCHECK(add_del_cache_.empty()); 730 DCHECK(add_del_cache_.empty());
709 DCHECK(sub_del_cache_.empty()); 731 DCHECK(sub_del_cache_.empty());
710 DCHECK(add_prefixes_.empty()); 732 DCHECK(add_prefixes_.empty());
711 DCHECK(sub_prefixes_.empty()); 733 DCHECK(sub_prefixes_.empty());
712 DCHECK(add_hashes_.empty()); 734 DCHECK(add_hashes_.empty());
(...skipping 41 matching lines...) Expand 10 before | Expand all | Expand 10 after
754 776
755 return OnCorruptDatabase(); 777 return OnCorruptDatabase();
756 } 778 }
757 779
758 file_.swap(file); 780 file_.swap(file);
759 new_file_.swap(new_file); 781 new_file_.swap(new_file);
760 return true; 782 return true;
761 } 783 }
762 784
763 bool SafeBrowsingStoreFile::FinishChunk() { 785 bool SafeBrowsingStoreFile::FinishChunk() {
786 DCHECK(CalledOnValidThread());
787
764 if (!add_prefixes_.size() && !sub_prefixes_.size() && 788 if (!add_prefixes_.size() && !sub_prefixes_.size() &&
765 !add_hashes_.size() && !sub_hashes_.size()) 789 !add_hashes_.size() && !sub_hashes_.size())
766 return true; 790 return true;
767 791
768 ChunkHeader header; 792 ChunkHeader header;
769 header.add_prefix_count = add_prefixes_.size(); 793 header.add_prefix_count = add_prefixes_.size();
770 header.sub_prefix_count = sub_prefixes_.size(); 794 header.sub_prefix_count = sub_prefixes_.size();
771 header.add_hash_count = add_hashes_.size(); 795 header.add_hash_count = add_hashes_.size();
772 header.sub_hash_count = sub_hashes_.size(); 796 header.sub_hash_count = sub_hashes_.size();
773 if (!WriteItem(header, new_file_.get(), NULL)) 797 if (!WriteItem(header, new_file_.get(), NULL))
774 return false; 798 return false;
775 799
776 if (!WriteContainer(add_prefixes_, new_file_.get(), NULL) || 800 if (!WriteContainer(add_prefixes_, new_file_.get(), NULL) ||
777 !WriteContainer(sub_prefixes_, new_file_.get(), NULL) || 801 !WriteContainer(sub_prefixes_, new_file_.get(), NULL) ||
778 !WriteContainer(add_hashes_, new_file_.get(), NULL) || 802 !WriteContainer(add_hashes_, new_file_.get(), NULL) ||
779 !WriteContainer(sub_hashes_, new_file_.get(), NULL)) 803 !WriteContainer(sub_hashes_, new_file_.get(), NULL))
780 return false; 804 return false;
781 805
782 ++chunks_written_; 806 ++chunks_written_;
783 807
784 // Clear everything to save memory. 808 // Clear everything to save memory.
785 return ClearChunkBuffers(); 809 return ClearChunkBuffers();
786 } 810 }
787 811
788 bool SafeBrowsingStoreFile::DoUpdate( 812 bool SafeBrowsingStoreFile::DoUpdate(
789 safe_browsing::PrefixSetBuilder* builder, 813 safe_browsing::PrefixSetBuilder* builder,
790 std::vector<SBAddFullHash>* add_full_hashes_result) { 814 std::vector<SBAddFullHash>* add_full_hashes_result) {
815 DCHECK(CalledOnValidThread());
791 DCHECK(file_.get() || empty_); 816 DCHECK(file_.get() || empty_);
792 DCHECK(new_file_.get()); 817 DCHECK(new_file_.get());
793 CHECK(builder); 818 CHECK(builder);
794 CHECK(add_full_hashes_result); 819 CHECK(add_full_hashes_result);
795 820
796 // Rewind the temporary storage. 821 // Rewind the temporary storage.
797 if (!FileRewind(new_file_.get())) 822 if (!FileRewind(new_file_.get()))
798 return false; 823 return false;
799 824
800 // Get chunk file's size for validating counts. 825 // Get chunk file's size for validating counts.
(...skipping 227 matching lines...) Expand 10 before | Expand all | Expand 10 after
1028 // Record counts before swapping to caller. 1053 // Record counts before swapping to caller.
1029 UMA_HISTOGRAM_COUNTS("SB2.AddPrefixes", add_prefix_count); 1054 UMA_HISTOGRAM_COUNTS("SB2.AddPrefixes", add_prefix_count);
1030 UMA_HISTOGRAM_COUNTS("SB2.SubPrefixes", sub_prefix_count); 1055 UMA_HISTOGRAM_COUNTS("SB2.SubPrefixes", sub_prefix_count);
1031 1056
1032 return true; 1057 return true;
1033 } 1058 }
1034 1059
1035 bool SafeBrowsingStoreFile::FinishUpdate( 1060 bool SafeBrowsingStoreFile::FinishUpdate(
1036 safe_browsing::PrefixSetBuilder* builder, 1061 safe_browsing::PrefixSetBuilder* builder,
1037 std::vector<SBAddFullHash>* add_full_hashes_result) { 1062 std::vector<SBAddFullHash>* add_full_hashes_result) {
1063 DCHECK(CalledOnValidThread());
1038 DCHECK(builder); 1064 DCHECK(builder);
1039 DCHECK(add_full_hashes_result); 1065 DCHECK(add_full_hashes_result);
1040 1066
1041 if (!DoUpdate(builder, add_full_hashes_result)) { 1067 if (!DoUpdate(builder, add_full_hashes_result)) {
1042 CancelUpdate(); 1068 CancelUpdate();
1043 return false; 1069 return false;
1044 } 1070 }
1045 1071
1046 DCHECK(!new_file_.get()); 1072 DCHECK(!new_file_.get());
1047 DCHECK(!file_.get()); 1073 DCHECK(!file_.get());
1048 1074
1049 return Close(); 1075 return Close();
1050 } 1076 }
1051 1077
1052 bool SafeBrowsingStoreFile::CancelUpdate() { 1078 bool SafeBrowsingStoreFile::CancelUpdate() {
1079 DCHECK(CalledOnValidThread());
1053 bool ret = Close(); 1080 bool ret = Close();
1054 1081
1055 // Delete stale staging file. 1082 // Delete stale staging file.
1056 const base::FilePath new_filename = TemporaryFileForFilename(filename_); 1083 const base::FilePath new_filename = TemporaryFileForFilename(filename_);
1057 base::DeleteFile(new_filename, false); 1084 base::DeleteFile(new_filename, false);
1058 1085
1059 return ret; 1086 return ret;
1060 } 1087 }
1061 1088
1062 void SafeBrowsingStoreFile::SetAddChunk(int32 chunk_id) { 1089 void SafeBrowsingStoreFile::SetAddChunk(int32 chunk_id) {
1090 DCHECK(CalledOnValidThread());
1063 add_chunks_cache_.insert(chunk_id); 1091 add_chunks_cache_.insert(chunk_id);
1064 } 1092 }
1065 1093
1066 bool SafeBrowsingStoreFile::CheckAddChunk(int32 chunk_id) { 1094 bool SafeBrowsingStoreFile::CheckAddChunk(int32 chunk_id) {
1095 DCHECK(CalledOnValidThread());
1067 return add_chunks_cache_.count(chunk_id) > 0; 1096 return add_chunks_cache_.count(chunk_id) > 0;
1068 } 1097 }
1069 1098
1070 void SafeBrowsingStoreFile::GetAddChunks(std::vector<int32>* out) { 1099 void SafeBrowsingStoreFile::GetAddChunks(std::vector<int32>* out) {
1100 DCHECK(CalledOnValidThread());
1071 out->clear(); 1101 out->clear();
1072 out->insert(out->end(), add_chunks_cache_.begin(), add_chunks_cache_.end()); 1102 out->insert(out->end(), add_chunks_cache_.begin(), add_chunks_cache_.end());
1073 } 1103 }
1074 1104
1075 void SafeBrowsingStoreFile::SetSubChunk(int32 chunk_id) { 1105 void SafeBrowsingStoreFile::SetSubChunk(int32 chunk_id) {
1106 DCHECK(CalledOnValidThread());
1076 sub_chunks_cache_.insert(chunk_id); 1107 sub_chunks_cache_.insert(chunk_id);
1077 } 1108 }
1078 1109
1079 bool SafeBrowsingStoreFile::CheckSubChunk(int32 chunk_id) { 1110 bool SafeBrowsingStoreFile::CheckSubChunk(int32 chunk_id) {
1111 DCHECK(CalledOnValidThread());
1080 return sub_chunks_cache_.count(chunk_id) > 0; 1112 return sub_chunks_cache_.count(chunk_id) > 0;
1081 } 1113 }
1082 1114
1083 void SafeBrowsingStoreFile::GetSubChunks(std::vector<int32>* out) { 1115 void SafeBrowsingStoreFile::GetSubChunks(std::vector<int32>* out) {
1116 DCHECK(CalledOnValidThread());
1084 out->clear(); 1117 out->clear();
1085 out->insert(out->end(), sub_chunks_cache_.begin(), sub_chunks_cache_.end()); 1118 out->insert(out->end(), sub_chunks_cache_.begin(), sub_chunks_cache_.end());
1086 } 1119 }
1087 1120
1088 void SafeBrowsingStoreFile::DeleteAddChunk(int32 chunk_id) { 1121 void SafeBrowsingStoreFile::DeleteAddChunk(int32 chunk_id) {
1122 DCHECK(CalledOnValidThread());
1089 add_del_cache_.insert(chunk_id); 1123 add_del_cache_.insert(chunk_id);
1090 } 1124 }
1091 1125
1092 void SafeBrowsingStoreFile::DeleteSubChunk(int32 chunk_id) { 1126 void SafeBrowsingStoreFile::DeleteSubChunk(int32 chunk_id) {
1127 DCHECK(CalledOnValidThread());
1093 sub_del_cache_.insert(chunk_id); 1128 sub_del_cache_.insert(chunk_id);
1094 } 1129 }
1095 1130
1096 // static 1131 // static
1097 bool SafeBrowsingStoreFile::DeleteStore(const base::FilePath& basename) { 1132 bool SafeBrowsingStoreFile::DeleteStore(const base::FilePath& basename) {
1098 if (!base::DeleteFile(basename, false) && 1133 if (!base::DeleteFile(basename, false) &&
1099 base::PathExists(basename)) { 1134 base::PathExists(basename)) {
1100 NOTREACHED(); 1135 NOTREACHED();
1101 return false; 1136 return false;
1102 } 1137 }
1103 1138
1104 const base::FilePath new_filename = TemporaryFileForFilename(basename); 1139 const base::FilePath new_filename = TemporaryFileForFilename(basename);
1105 if (!base::DeleteFile(new_filename, false) && 1140 if (!base::DeleteFile(new_filename, false) &&
1106 base::PathExists(new_filename)) { 1141 base::PathExists(new_filename)) {
1107 NOTREACHED(); 1142 NOTREACHED();
1108 return false; 1143 return false;
1109 } 1144 }
1110 1145
1111 // With SQLite support gone, one way to get to this code is if the 1146 // With SQLite support gone, one way to get to this code is if the
1112 // existing file is a SQLite file. Make sure the journal file is 1147 // existing file is a SQLite file. Make sure the journal file is
1113 // also removed. 1148 // also removed.
1114 const base::FilePath journal_filename( 1149 const base::FilePath journal_filename(
1115 basename.value() + FILE_PATH_LITERAL("-journal")); 1150 basename.value() + FILE_PATH_LITERAL("-journal"));
1116 if (base::PathExists(journal_filename)) 1151 if (base::PathExists(journal_filename))
1117 base::DeleteFile(journal_filename, false); 1152 base::DeleteFile(journal_filename, false);
1118 1153
1119 return true; 1154 return true;
1120 } 1155 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698