(Reland) Remove about:srcdoc url conversion.

content/browser/frame_host/navigation_entry_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/navigation_entry_impl.h"
 
 #include <stddef.h>
 
 #include <queue>
 #include <utility>
@@ skipping 857 matching lines @@
     FrameTreeNode* frame_tree_node) const {
   std::map<std::string, bool> names;
   NavigationEntryImpl::TreeNode* tree_node = FindFrameEntry(frame_tree_node);
   if (tree_node) {
     // Return the names of all immediate children.
     for (TreeNode* child : tree_node->children) {
       // Keep track of whether we would be loading about:blank, since the
       // renderer should be allowed to just commit the initial blank frame if
       // that was the default URL.  PageState doesn't matter there, because
       // content injected into about:blank frames doesn't use it.
-      //
-      // Be careful not to include iframe srcdoc URLs in this check, which do
-      // need their PageState.  The committed URL in that case gets rewritten to
-      // about:blank, but we can detect it via the PageState's URL.
-      //
-      // See https://crbug.com/657896 for details.
-      bool is_about_blank = false;
-      ExplodedPageState exploded_page_state;
-      if (DecodePageState(child->frame_entry->page_state().ToEncodedData(),
-                          &exploded_page_state)) {
-        ExplodedFrameState frame_state = exploded_page_state.top;
-        if (UTF16ToUTF8(frame_state.url_string.string()) == url::kAboutBlankURL)
-          is_about_blank = true;
-      }
-
-      names[child->frame_entry->frame_unique_name()] = is_about_blank;
+      names[child->frame_entry->frame_unique_name()] =
+          child->frame_entry->url() == url::kAboutBlankURL;

[arthursonzogni, 2016/12/01 09:39:51]

Too bad we can't keep this...
What no longer applies to about:srcdoc stills applies to about:blank/#identifier
for instance.

The browser thinks the subframe has navigated to about:blank, but the renderer
knows that the real URL is about:blank/#identifier.

This caused a test to fail:
fast/history/history-length-append-subframe-with-hash.html

[Charlie Reis, 2016/12/02 00:20:32]

Hmm, I think your new version of this is problematic.

This GetSubframeUniqueNames method intentionally keeps track of whether the
subframe will be about:blank, because those frames must be allowed to load
synchronously (for the parent page to be able to immediately inject code into
them).

If we say false for about:blank/#identifier, then that frame will be loaded
asynchronously, leading to bugs like https://crbug.com/626416 and
https://crbug.com/657896 (which I just wrote a postmortem for).  :)

However, I also see why the synchronous approach would cause the test to fail--
saying true here would make the frame load about:blank rather than
about:blank#identifier.

Maybe it's time to finish up https://crbug.com/639842 and pass the full
PageState for any same-process frames, rather than just a boolean of whether
it's about:blank?  I think that would unblock you.

Let me know if you have other ideas in the meantime that don't require as big a
change but avoid the issue I describe above.  (I suppose we could stop rewriting
about:blank/#identifier?  Not sure what the implications would be, but that
could be a net positive change-- these sorts of differences between browser and
renderer do cause bugs, and they were likely overlooked when this rewriting was
introduced.)

[Charlie Reis, 2016/12/02 05:12:51]

Ah, it occurs to me that this bug is present today!  (I was looking at the diffs
between patchsets and not the overall diff.)  Guess we've discovered a bug to
fix...

As a result, this CL LGTM and I'll follow up on the asynchronous
about:blank/#identifier issue.

     }
   }
   return names;
 }
 
 void NavigationEntryImpl::ClearStaleFrameEntriesForNewFrame(
     FrameTreeNode* frame_tree_node) {
   DCHECK(!frame_tree_node->IsMainFrame());
 
   NavigationEntryImpl::TreeNode* node = nullptr;
@@ skipping 52 matching lines @@
       return node;
 
     // Enqueue any children and keep looking.
     for (auto* child : node->children)
       work_queue.push(child);
   }
   return nullptr;
 }
 
 }  // namespace content