Add UMA to measure feasibility of making unique names immutable

content/renderer/render_frame_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/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 2948 matching lines @@
   // |report_frame_name_changes| is set (used by <webview>).  If needed, this
   // can be optimized further by only sending the update if there are any
   // remote frames in the frame tree, or delaying and batching up IPCs if
   // updates are happening too frequently.
   if (SiteIsolationPolicy::AreCrossProcessFramesPossible() ||
       render_view_->renderer_preferences_.report_frame_name_changes) {
     Send(new FrameHostMsg_DidChangeName(
         routing_id_, base::UTF16ToUTF8(base::StringPiece16(name)),
         base::UTF16ToUTF8(base::StringPiece16(unique_name))));
   }
+
+  if (!committed_first_load_) {

[Charlie Reis, 2016/10/01 00:05:12]

nit: No braces.

[dcheng, 2016/10/01 20:25:14]

Done.

+    name_changed_before_first_commit_ = true;
+  }
 }
 
 void RenderFrameImpl::didEnforceInsecureRequestPolicy(
     blink::WebInsecureRequestPolicy policy) {
   Send(new FrameHostMsg_EnforceInsecureRequestPolicy(routing_id_, policy));
 }
 
 void RenderFrameImpl::didUpdateToUniqueOrigin(
     bool is_potentially_trustworthy_unique_origin) {
   Send(new FrameHostMsg_UpdateToUniqueOrigin(
@@ skipping 365 matching lines @@
 }
 
 void RenderFrameImpl::didCommitProvisionalLoad(
     blink::WebLocalFrame* frame,
     const blink::WebHistoryItem& item,
     blink::WebHistoryCommitType commit_type) {
   TRACE_EVENT2("navigation,rail", "RenderFrameImpl::didCommitProvisionalLoad",
                "id", routing_id_,
                "url", GetLoadingUrl().possibly_invalid_spec());
   DCHECK_EQ(frame_, frame);
+
+  // TODO(dcheng): Remove this UMA once we have enough measurements.
+  if (!committed_first_load_ && !current_history_item_.isNull()) {

[Charlie Reis, 2016/10/01 00:05:12]

Why isn't current_history_item_ enough (here and above)?  Seems like they're
both only set in this method.

[dcheng, 2016/10/01 20:25:14]

I added a comment to try to make the purpose clearer.

+    if (!IsMainFrame()) {
+      UMA_HISTOGRAM_BOOLEAN(
+          "SessionRestore.SubFrameUniqueNameChangedBeforeFirstCommit",
+          name_changed_before_first_commit_);
+    }
+    committed_first_load_ = true;
+  }
+
   DocumentState* document_state =
       DocumentState::FromDataSource(frame->dataSource());
   NavigationStateImpl* navigation_state =
       static_cast<NavigationStateImpl*>(document_state->navigation_state());
   WebURLResponseExtraDataImpl* extra_data =
       GetExtraDataFromResponse(frame->dataSource()->response());
   // Only update the Lo-Fi and effective connection type states for new main
   // frame documents. Subframes inherit from the main frame and should not
   // change at commit time.
   if (is_main_frame_ && !navigation_state->WasWithinSamePage()) {
@@ skipping 3071 matching lines @@
   // event target. Potentially a Pepper plugin will receive the event.
   // In order to tell whether a plugin gets the last mouse event and which it
   // is, we set |pepper_last_mouse_event_target_| to null here. If a plugin gets
   // the event, it will notify us via DidReceiveMouseEvent() and set itself as
   // |pepper_last_mouse_event_target_|.
   pepper_last_mouse_event_target_ = nullptr;
 #endif
 }
 
 }  // namespace content