4 files changed, 31 insertions, 18 deletions

diff --git a/media/mp4/box_definitions.cc b/media/mp4/box_definitions.cc
index 9b64f48..aa179e1 100644
--- a/media/mp4/box_definitions.cc
+++ b/media/mp4/box_definitions.cc
@@ -225,8 +225,7 @@ FourCC SampleDescription::BoxType() const { return FOURCC_STSD; }
 bool SampleDescription::Parse(BoxReader* reader) {
   uint32 count;
   RCHECK(reader->SkipBytes(4) &&
-         reader->Read4(&count) &&
-         reader->ScanChildren());
+         reader->Read4(&count));
   video_entries.clear();
   audio_entries.clear();
 
diff --git a/media/mp4/box_reader.cc b/media/mp4/box_reader.cc
index b352664..1285ebf 100644
--- a/media/mp4/box_reader.cc
+++ b/media/mp4/box_reader.cc
@@ -163,8 +163,6 @@ bool BoxReader::ScanChildren() {
   scanned_ = true;
 
   bool err = false;
-  // TODO(strobe): Check or correct for multimap not inserting elements in
-  // consistent order.
   while (pos() < size()) {
     BoxReader child(&buf_[pos_], size_ - pos_);
     if (!child.ReadHeader(&err)) break;
diff --git a/media/mp4/box_reader.h b/media/mp4/box_reader.h
index 211d99a..967bf93 100644
--- a/media/mp4/box_reader.h
+++ b/media/mp4/box_reader.h
@@ -118,6 +118,7 @@ class MEDIA_EXPORT BoxReader : public BufferReader {
 
   // Read all children, regardless of FourCC. This is used from exactly one box,
   // corresponding to a rather significant inconsistency in the BMFF spec.
+  // Note that this method is mutually exclusive with ScanChildren().
   template<typename T> bool ReadAllChildren(
       std::vector<T>* children) WARN_UNUSED_RESULT;
 
@@ -185,19 +186,20 @@ bool BoxReader::MaybeReadChildren(std::vector<T>* children) {
 
 template<typename T>
 bool BoxReader::ReadAllChildren(std::vector<T>* children) {
-  DCHECK(scanned_);
-  DCHECK(children->empty());
-  RCHECK(!children_.empty());
-
-  children->resize(children_.size());
-  typename std::vector<T>::iterator child_itr = children->begin();
-  for (ChildMap::iterator itr = children_.begin();
-       itr != children_.end(); ++itr) {
-    RCHECK(child_itr->Parse(&itr->second));
-    ++child_itr;
+  DCHECK(!scanned_);
+  scanned_ = true;
+
+  bool err = false;
+  while (pos() < size()) {
+    BoxReader child_reader(&buf_[pos_], size_ - pos_);
+    if (!child_reader.ReadHeader(&err)) break;
+    T child;
+    RCHECK(child.Parse(&child_reader));
+    children->push_back(child);
+    pos_ += child_reader.size();
   }
-  children_.clear();
-  return true;
+
+  return !err;
 }
 
 }  // namespace mp4
diff --git a/media/mp4/box_reader_unittest.cc b/media/mp4/box_reader_unittest.cc
index da0a041..38fd896 100644
--- a/media/mp4/box_reader_unittest.cc
+++ b/media/mp4/box_reader_unittest.cc
@@ -145,7 +145,7 @@ TEST_F(BoxReaderTest, WrongFourCCTest) {
   EXPECT_TRUE(err);
 }
 
-TEST_F(BoxReaderTest, ChildrenTest) {
+TEST_F(BoxReaderTest, ScanChildrenTest) {
   std::vector<uint8> buf = GetBuf();
   bool err;
   scoped_ptr<BoxReader> reader(
@@ -160,12 +160,26 @@ TEST_F(BoxReaderTest, ChildrenTest) {
 
   std::vector<PsshBox> kids;
 
-  EXPECT_TRUE(reader->ReadAllChildren(&kids));
+  EXPECT_TRUE(reader->ReadChildren(&kids));
   EXPECT_EQ(2u, kids.size());
   kids.clear();
   EXPECT_FALSE(reader->ReadChildren(&kids));
   EXPECT_TRUE(reader->MaybeReadChildren(&kids));
 }
 
+TEST_F(BoxReaderTest, ReadAllChildrenTest) {
+  std::vector<uint8> buf = GetBuf();
+  // Modify buffer to exclude its last 'free' box
+  buf[3] = 0x38;
+  bool err;
+  scoped_ptr<BoxReader> reader(
+      BoxReader::ReadTopLevelBox(&buf[0], buf.size(), &err));
+
+  std::vector<PsshBox> kids;
+  EXPECT_TRUE(reader->SkipBytes(16) && reader->ReadAllChildren(&kids));
+  EXPECT_EQ(2u, kids.size());
+  EXPECT_EQ(kids[0].val, 0xdeadbeef);   // Ensure order is preserved
+}
+
 }  // namespace mp4
 }  // namespace media