Replicating LayoutDumpFlags across OOPIFs.

components/test_runner/test_runner.cc

Index: components/test_runner/test_runner.cc
diff --git a/components/test_runner/test_runner.cc b/components/test_runner/test_runner.cc
index b85bc61f1d4c151616a29c855daee6a2654960a0..cef9e0d72a10569807adc38cd0c4f4d88692fb79 100644
--- a/components/test_runner/test_runner.cc
+++ b/components/test_runner/test_runner.cc
@@ -1610,7 +1610,7 @@ void TestRunner::WorkQueue::ProcessWorkSoon() {
   if (!queue_.empty()) {
     // We delay processing queued work to avoid recursion problems.
     controller_->delegate_->PostTask(new WorkQueueTask(this));
-  } else if (!controller_->wait_until_done_) {
+  } else if (!controller_->layout_dump_flags_.wait_until_done()) {
     controller_->delegate_->TestFinished();
   }
 }
@@ -1642,7 +1642,8 @@ void TestRunner::WorkQueue::ProcessWork() {
       return;
   }
 
-  if (!controller_->wait_until_done_ && !controller_->topLoadingFrame())
+  if (!controller_->layout_dump_flags_.wait_until_done() &&
+      !controller_->topLoadingFrame())
     controller_->delegate_->TestFinished();
 }
 
@@ -1657,13 +1658,6 @@ TestRunner::TestRunner(TestInterfaces* interfaces)
       disable_notify_done_(false),
       web_history_item_count_(0),
       intercept_post_message_(false),
-      layout_dump_flags_(
-          false,  // dump_as_text
-          false,  // dump_child_frames_as_text
-          false,  // dump_as_markup
-          false,  // dump_child_frames_as_markup
-          false,  // dump_child_frame_scroll_positions
-          false),  // is_printing
       test_interfaces_(interfaces),
       delegate_(nullptr),
       web_view_(nullptr),
@@ -1704,7 +1698,7 @@ void TestRunner::Reset() {
   }
 
   top_loading_frame_ = nullptr;
-  wait_until_done_ = false;
+  layout_dump_flags_.Reset();
   wait_until_external_url_load_ = false;
   policy_delegate_enabled_ = false;
   policy_delegate_is_permissive_ = false;
@@ -1733,12 +1727,6 @@ void TestRunner::Reset() {
   }
 
   dump_editting_callbacks_ = false;
-  layout_dump_flags_.dump_as_text = false;
-  layout_dump_flags_.dump_as_markup = false;
-  generate_pixel_results_ = true;
-  layout_dump_flags_.dump_child_frame_scroll_positions = false;
-  layout_dump_flags_.dump_child_frames_as_text = false;
-  layout_dump_flags_.dump_child_frames_as_markup = false;
   dump_icon_changes_ = false;
   dump_as_audio_ = false;
   dump_frame_load_callbacks_ = false;
@@ -1758,7 +1746,6 @@ void TestRunner::Reset() {
   dump_navigation_policy_ = false;
   test_repaint_ = false;
   sweep_horizontally_ = false;
-  layout_dump_flags_.is_printing = false;
   midi_accessor_result_ = true;
   should_stay_on_page_after_handling_before_unload_ = false;
   should_dump_resource_priorities_ = false;
@@ -1802,11 +1789,13 @@ bool TestRunner::shouldDumpEditingCallbacks() const {
 }
 
 void TestRunner::setShouldDumpAsText(bool value) {
-  layout_dump_flags_.dump_as_text = value;
+  layout_dump_flags_.set_dump_as_text(value);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::setShouldDumpAsMarkup(bool value) {
-  layout_dump_flags_.dump_as_markup = value;
+  layout_dump_flags_.set_dump_as_markup(value);
+  OnLayoutDumpFlagsChanged();
 }
 
 bool TestRunner::shouldDumpAsCustomText() const {
@@ -1824,7 +1813,7 @@ void TestRunner::setCustomTextOutput(const std::string& text) {
 
 bool TestRunner::ShouldGeneratePixelResults() {
   CheckResponseMimeType();
-  return generate_pixel_results_;
+  return layout_dump_flags_.generate_pixel_results();
 }
 
 bool TestRunner::ShouldStayOnPageAfterHandlingBeforeUnload() const {
@@ -1833,7 +1822,8 @@ bool TestRunner::ShouldStayOnPageAfterHandlingBeforeUnload() const {
 
 
 void TestRunner::setShouldGeneratePixelResults(bool value) {
-  generate_pixel_results_ = value;
+  layout_dump_flags_.set_generate_pixel_results(value);
+  OnLayoutDumpFlagsChanged();
 }
 
 bool TestRunner::ShouldDumpAsAudio() const {
@@ -1849,6 +1839,11 @@ const LayoutDumpFlags& TestRunner::GetLayoutDumpFlags() {
   return layout_dump_flags_;
 }
 
+void TestRunner::ReplicateLayoutDumpFlagsChanges(
+    const base::DictionaryValue& changed_values) {
+  layout_dump_flags_.tracked_dictionary().ApplyUntrackedChanges(changed_values);
+}
+
 bool TestRunner::HasCustomTextDump(std::string* custom_text_dump) const {
   if (shouldDumpAsCustomText()) {
     *custom_text_dump = customDumpText();
@@ -1931,7 +1926,7 @@ bool TestRunner::shouldDumpSelectionRect() const {
 }
 
 bool TestRunner::isPrinting() const {
-  return layout_dump_flags_.is_printing;
+  return layout_dump_flags_.is_printing();
 }
 
 bool TestRunner::shouldWaitUntilExternalURLLoad() const {
@@ -1960,9 +1955,10 @@ WebFrame* TestRunner::topLoadingFrame() const {
 }
 
 void TestRunner::policyDelegateDone() {
-  DCHECK(wait_until_done_);
+  DCHECK(layout_dump_flags_.wait_until_done());
   delegate_->TestFinished();
-  wait_until_done_ = false;
+  layout_dump_flags_.set_wait_until_done(false);
+  OnLayoutDumpFlagsChanged();
 }
 
 bool TestRunner::policyDelegateEnabled() const {
@@ -2062,7 +2058,8 @@ void TestRunner::NotifyDone() {
 }
 
 void TestRunner::WaitUntilDone() {
-  wait_until_done_ = true;
+  layout_dump_flags_.set_wait_until_done(true);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::QueueBackNavigation(int how_far_back) {
@@ -2195,7 +2192,8 @@ void TestRunner::SetCustomPolicyDelegate(gin::Arguments* args) {
 void TestRunner::WaitForPolicyDelegate() {
   policy_delegate_enabled_ = true;
   policy_delegate_should_notify_done_ = true;
-  wait_until_done_ = true;
+  layout_dump_flags_.set_wait_until_done(true);
+  OnLayoutDumpFlagsChanged();
 }
 
 int TestRunner::WindowCount() {
@@ -2640,30 +2638,36 @@ void TestRunner::DumpEditingCallbacks() {
 }
 
 void TestRunner::DumpAsMarkup() {
-  layout_dump_flags_.dump_as_markup = true;
-  generate_pixel_results_ = false;
+  layout_dump_flags_.set_dump_as_markup(true);
+  layout_dump_flags_.set_generate_pixel_results(false);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::DumpAsText() {
-  layout_dump_flags_.dump_as_text = true;
-  generate_pixel_results_ = false;
+  layout_dump_flags_.set_dump_as_text(true);
+  layout_dump_flags_.set_generate_pixel_results(false);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::DumpAsTextWithPixelResults() {
-  layout_dump_flags_.dump_as_text = true;
-  generate_pixel_results_ = true;
+  layout_dump_flags_.set_dump_as_text(true);
+  layout_dump_flags_.set_generate_pixel_results(true);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::DumpChildFrameScrollPositions() {
-  layout_dump_flags_.dump_child_frame_scroll_positions = true;
+  layout_dump_flags_.set_dump_child_frame_scroll_positions(true);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::DumpChildFramesAsMarkup() {
-  layout_dump_flags_.dump_child_frames_as_markup = true;
+  layout_dump_flags_.set_dump_child_frames_as_markup(true);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::DumpChildFramesAsText() {
-  layout_dump_flags_.dump_child_frames_as_text = true;
+  layout_dump_flags_.set_dump_child_frames_as_text(true);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::DumpIconChanges() {
@@ -2762,11 +2766,13 @@ void TestRunner::DumpSelectionRect() {
 }
 
 void TestRunner::SetPrinting() {
-  layout_dump_flags_.is_printing = true;
+  layout_dump_flags_.set_is_printing(true);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::ClearPrinting() {
-  layout_dump_flags_.is_printing = false;
+  layout_dump_flags_.set_is_printing(false);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::SetShouldStayOnPageAfterHandlingBeforeUnload(bool value) {
@@ -3020,6 +3026,13 @@ void TestRunner::CapturePixelsAsyncThen(v8::Local<v8::Function> callback) {
                                         base::Passed(&task)));
 }
 
+void TestRunner::OnLayoutDumpFlagsChanged() {
+  if (layout_dump_flags_.tracked_dictionary().changed_values().size() == 0)

[dcheng, 2016/03/08 23:09:37]

.empty()?

[Łukasz Anforowicz, 2016/03/10 22:24:21]

Done.

+    return;
+
+  delegate_->OnLayoutDumpFlagsChanged(&layout_dump_flags_);

[dcheng, 2016/03/08 23:09:37]

Nit: pass a const ref, I don't see a place that actually needs this to be an
optional value.

[Łukasz Anforowicz, 2016/03/10 22:24:21]

Done.  I've moved responsibility for deciding when to reset change tracking from
BlinkTestRunner to TestRunner.  I think this is ok, but please read the
explanation below to sanity check.

This cannot be "const", because BlinkTestRunner::OnLayoutDumpFlagsChanged
currently calls
|layout_dump_flags->tracked_dictionary().ResetChangeTracking();|.  Looking at it
again, I am not sure why I thought that ResetChangeTracking should be called by
BlinkTestRunner, rather than by TestRunner.  It felt safer to only reset change
tracking at the exact moment when sending the changes to the browser.  OTOH, we
only skip changes when !is_main_window (and the changes prior to that will be
cloberred anyway by ResetChangeTracking called from LayoutTestFlags::Reset).

+}
+
 void TestRunner::ForceNextWebGLContextCreationToFail() {
   if (web_view_)
     web_view_->forceNextWebGLContextCreationToFail();
@@ -3137,27 +3150,36 @@ void TestRunner::LocationChangeDone() {
   // No more new work after the first complete load.
   work_queue_.set_frozen(true);
 
-  if (!wait_until_done_)
+  if (!layout_dump_flags_.wait_until_done())
     work_queue_.ProcessWorkSoon();
 }
 
 void TestRunner::CheckResponseMimeType() {
   // Text output: the test page can request different types of output which we
   // handle here.
-  if (!layout_dump_flags_.dump_as_text) {
-    std::string mimeType =
-        web_view_->mainFrame()->dataSource()->response().mimeType().utf8();
-    if (mimeType == "text/plain") {
-      layout_dump_flags_.dump_as_text = true;
-      generate_pixel_results_ = false;
-    }
-  }
+
+  if (layout_dump_flags_.dump_as_text())
+    return;
+
+  WebDataSource* data_source = web_view_->mainFrame()->dataSource();
+  if (!data_source)
+    return;
+
+  std::string mimeType = data_source->response().mimeType().utf8();
+  if (mimeType != "text/plain")
+    return;
+
+  layout_dump_flags_.set_dump_as_text(true);
+  layout_dump_flags_.set_generate_pixel_results(false);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::CompleteNotifyDone() {
-  if (wait_until_done_ && !topLoadingFrame() && work_queue_.is_empty())
+  if (layout_dump_flags_.wait_until_done() && !topLoadingFrame() &&
+      work_queue_.is_empty())
     delegate_->TestFinished();
-  wait_until_done_ = false;
+  layout_dump_flags_.set_wait_until_done(false);
+  OnLayoutDumpFlagsChanged();
 }
 
 void TestRunner::DidAcquirePointerLockInternal() {