Replicating WebFrame::uniqueName across renderers.

content/common/frame_replication_state.h

 // Copyright 2014 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.
 
 #ifndef CONTENT_COMMON_FRAME_REPLICATION_STATE_H_
 #define CONTENT_COMMON_FRAME_REPLICATION_STATE_H_
 
 #include "content/common/content_export.h"
 #include "url/origin.h"
 
 namespace blink {
 enum class WebTreeScopeType;
 enum class WebSandboxFlags;
 }
 
 namespace content {
 
 // This structure holds information that needs to be replicated between a
 // RenderFrame and any of its associated RenderFrameProxies.
 struct CONTENT_EXPORT FrameReplicationState {
   FrameReplicationState();
   FrameReplicationState(blink::WebTreeScopeType scope,
                         const std::string& name,
+                        const std::string& unique_name,
                         blink::WebSandboxFlags sandbox_flags,
                         bool should_enforce_strict_mixed_content_checking);
   ~FrameReplicationState();
 
   // Current origin of the frame. This field is updated whenever a frame
   // navigation commits.
   //
   // TODO(alexmos): For now, |origin| updates are immediately sent to all frame
   // proxies when in --site-per-process mode. This isn't ideal, since Blink
   // typically needs a proxy's origin only when performing security checks on
@@ skipping 10 matching lines @@
   //
   // When a parent frame updates an <iframe>'s sandbox attribute via
   // JavaScript, |sandbox_flags| are updated only after the child frame commits
   // a navigation that makes the updated flags take effect.  This is also the
   // point at which updates are sent to proxies (see
   // CommitPendingSandboxFlags()). The proxies need updated flags so that they
   // can be inherited properly if a proxy ever becomes a parent of a local
   // frame.
   blink::WebSandboxFlags sandbox_flags;
 
-  // The assigned name of the frame. This name can be empty, unlike the unique

[Charlie Reis, 2016/01/27 23:48:49]

Why isn't this true anymore?

[Łukasz Anforowicz, 2016/01/28 01:21:19]

AFAICT it was never true.  If you look at FrameTree::setName, you see that for
main frame we simply assign |m_uniqueName = name|.  And the main frame initially
uses nullAtom as its name (i.e. see WebViewImpl::setMainFrame method).

[Charlie Reis, 2016/02/11 22:02:14]

Acknowledged.

-  // name generated internally in the DOM tree.
+  // The assigned name of the frame.
   //
   // |name| is set when a new child frame is created using the value of the
   // <iframe> element's "name" attribute (see
   // RenderFrameHostImpl::OnCreateChildFrame), and it is updated dynamically
   // whenever a frame sets its window.name.
   //
   // |name| updates are immediately sent to all frame proxies (when in
   // --site-per-process mode), so that other frames can look up or navigate a
   // frame using its updated name (e.g., using window.open(url, frame_name)).
   std::string name;
 
+  // Unique name of the frame.
+  // https://crbug.com/576969: This isn't really "unique" today.

[Charlie Reis, 2016/01/27 23:48:49]

I don't think it makes sense to document this here.  It'll likely just become a
stale comment once the bug is fixed.

[Łukasz Anforowicz, 2016/01/28 01:21:19]

Done.
Hmmm... should there be a tool that complains when there are crbug comments in
Chromium's source code that refer to bugs that have been marked as fixed?

[alexmos, 2016/02/10 00:46:45]

I don't think that's always undesirable.  For example, when I'm writing a test,
I often mention which bug it's for, and I think it's fine to refer to a fixed
bug in that case.

[Łukasz Anforowicz, 2016/02/10 22:10:50]

Acknowledged.  Maybe just pay attention to bugs on TODO lines?  I get 1081 bugs
with the cmdline below and some of them are stale (i.e. issue 8123):

$ git grep 'TODO.*crbug.com/[0-9]' | \
    sed -e 's:^.*crbug.com/\([0-9]\+\).*$:\1:g' | \
    sort | uniq

[Charlie Reis, 2016/02/11 22:02:14]

Yeah, I've wondered this myself sometimes.  Finding TODOs with closed bug
numbers could be useful.  What would you do with that data?  It seems more like
something to process in the background and email the author of the TODO (and/or
the person who closed the bug).

[Łukasz Anforowicz, 2016/02/11 23:23:55]

I thought that the bug could be marked with a label like "StaleTodo" and
commented with the location of the TODO.  The label should help avoid commenting
on the same bug twice.

This also sounds like something where a quick ad-hoc script would do a job more
or less fine, but where potentially a lot of effort is needed to do things the
right way (host the script somewhere, run it automatically, maintain
write-permissions for the script, make sure that this is actually useful and
that somebody follows-up on the marked bugs).  Maybe I should try to pitch the
idea on chromium-dev.

+  std::string unique_name;
+
   // Whether the frame is in a document tree or a shadow tree, per the Shadow
   // DOM spec: https://w3c.github.io/webcomponents/spec/shadow/
   // Note: This should really be const, as it can never change once a frame is
   // created. However, making it const makes it a pain to embed into IPC message
   // params: having a const member implicitly deletes the copy assignment
   // operator.
   blink::WebTreeScopeType scope;
 
   // True if a frame's current document should strictly block all mixed
   // content. Updates are immediately sent to all frame proxies when
   // frames live in different processes.
   bool should_enforce_strict_mixed_content_checking;
 
   // TODO(alexmos): Eventually, this structure can also hold other state that
   // needs to be replicated, such as frame sizing info.
 };
 
 }  // namespace content
 
 #endif  // CONTENT_COMMON_FRAME_REPLICATION_STATE_H_