[SyncFS] Support Delete operation on LevelDBWrapper::Iterator

[SyncFS] Support Delete operation on LevelDBWrapper::Iterator

After running Delete() method, the iterator is expected to index
the next entry, if exists.


BUG=347425
TEST=./unit_tests --gtest_filter="LevelDBWrapper*"

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

[peria, 2014-08-12 02:08:33]

PTL.

[tzik, 2014-08-12 03:37:32]

https://codereview.chromium.org/461823002/diff/1/chrome/browser/sync_file_sys...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc (right):

https://codereview.chromium.org/461823002/diff/1/chrome/browser/sync_file_sys...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc:75:
Seek(key_str);
Can this be like below?
  map_iterator->second = Transaction(DELETE_OPERATION, std::string());
  AdvanceIterators();

[peria, 2014-08-12 05:46:36]

https://codereview.chromium.org/461823002/diff/1/chrome/browser/sync_file_sys...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc (right):

https://codereview.chromium.org/461823002/diff/1/chrome/browser/sync_file_sys...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc:75:
Seek(key_str);
On 2014/08/12 03:37:32, tzik wrote:
> Can this be like below?
>   map_iterator->second = Transaction(DELETE_OPERATION, std::string());
>   AdvanceIterators();

As we chatted offline, it does not work if |db_->pending_| does not
have an element with the key.

I (remove and) insert the element, instead.  How is it?

[tzik, 2014-08-12 05:56:46]

https://codereview.chromium.org/461823002/diff/20001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc (right):

https://codereview.chromium.org/461823002/diff/20001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc:78:
Transaction(DELETE_OPERATION, std::string()))).first;
Since we already have the right place to insert, how about doing like this?

Transaction deletion(DELETE_OPERATION, std::string());
std::pair<iterator, bool> inserted = db_->pending_.insert(map_iterator_,
std::make_pair(key_str, deletion));
map_iterator_ = inserted.first;
if (!inserted.second)
  map_iterator_->second = deletion;

[peria, 2014-08-12 06:04:34]

https://codereview.chromium.org/461823002/diff/20001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc (right):

https://codereview.chromium.org/461823002/diff/20001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc:78:
Transaction(DELETE_OPERATION, std::string()))).first;
On 2014/08/12 05:56:46, tzik wrote:
> Since we already have the right place to insert, how about doing like this?
> 
> Transaction deletion(DELETE_OPERATION, std::string());
> std::pair<iterator, bool> inserted = db_->pending_.insert(map_iterator_,
> std::make_pair(key_str, deletion));
> map_iterator_ = inserted.first;
> if (!inserted.second)
>   map_iterator_->second = deletion;

Done.

[tzik, 2014-08-12 06:28:33]

https://codereview.chromium.org/461823002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc (right):

https://codereview.chromium.org/461823002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc:76:
db_->pending_.insert(std::make_pair(key_str, deletion));
Can we put previous iterator as a hint like this?
insert(map_iterator_, std::make_pair(...))

[peria, 2014-08-12 06:41:49]

https://codereview.chromium.org/461823002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc (right):

https://codereview.chromium.org/461823002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc:76:
db_->pending_.insert(std::make_pair(key_str, deletion));
On 2014/08/12 06:28:33, tzik wrote:
> Can we put previous iterator as a hint like this?
> insert(map_iterator_, std::make_pair(...))

We can, but if we can add a hint, insert() returns an iterator, not a pair.
We need a pair to detect the element is newly added or not,
so we cannot use the hint.
http://www.cplusplus.com/reference/map/map/insert/

[tzik, 2014-08-12 07:32:37]

https://codereview.chromium.org/461823002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc (right):

https://codereview.chromium.org/461823002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc:76:
db_->pending_.insert(std::make_pair(key_str, deletion));
On 2014/08/12 06:41:48, peria wrote:
> On 2014/08/12 06:28:33, tzik wrote:
> > Can we put previous iterator as a hint like this?
> > insert(map_iterator_, std::make_pair(...))
> 
> We can, but if we can add a hint, insert() returns an iterator, not a pair.
> We need a pair to detect the element is newly added or not,
> so we cannot use the hint.
> http://www.cplusplus.com/reference/map/map/insert/

Ah, I wasn't aware that.
How about doing like below?
map_iterator_ = insert(map_iterator, std::make_pair(...));
map_iterator_->second = Transaction();

[peria, 2014-08-12 07:44:35]

https://codereview.chromium.org/461823002/diff/40001/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc (right):

https://codereview.chromium.org/461823002/diff/40001/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc:76:
db_->pending_.insert(std::make_pair(key_str, deletion));
On 2014/08/12 07:32:36, tzik wrote:
> On 2014/08/12 06:41:48, peria wrote:
> > On 2014/08/12 06:28:33, tzik wrote:
> > > Can we put previous iterator as a hint like this?
> > > insert(map_iterator_, std::make_pair(...))
> > 
> > We can, but if we can add a hint, insert() returns an iterator, not a pair.
> > We need a pair to detect the element is newly added or not,
> > so we cannot use the hint.
> > http://www.cplusplus.com/reference/map/map/insert/
> 
> Ah, I wasn't aware that.
> How about doing like below?
> map_iterator_ = insert(map_iterator, std::make_pair(...));
> map_iterator_->second = Transaction();

Done.

[tzik, 2014-08-12 08:01:32]

lgtm

[peria, 2014-08-12 08:02:53]

The CQ bit was checked by peria@chromium.org

[commit-bot: I haz the power, 2014-08-12 08:04:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/peria@chromium.org/461823002/60001

[commit-bot: I haz the power, 2014-08-12 15:47:58]

Change committed as 288991