DevTools: Introduce Page.getManifest remote debugging protocol method.

content/renderer/manifest/manifest_manager.cc

 // 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.
 
 #include "content/renderer/manifest/manifest_manager.h"
 
 #include "base/bind.h"
 #include "base/strings/nullable_string16.h"
 #include "content/common/manifest_manager_messages.h"
 #include "content/public/renderer/render_frame.h"
@@ skipping 42 matching lines @@
   Send(new ManifestManagerHostMsg_HasManifestResponse(
       routing_id(), request_id, has_manifest));
 }
 
 void ManifestManager::OnRequestManifest(int request_id) {
   GetManifest(base::Bind(&ManifestManager::OnRequestManifestComplete,
                          base::Unretained(this), request_id));
 }
 
 void ManifestManager::OnRequestManifestComplete(
-    int request_id, const Manifest& manifest) {
+    int request_id, const Manifest& manifest,
+    const ManifestDebugInfo&) {
   // When sent via IPC, the Manifest must follow certain security rules.
   Manifest ipc_manifest = manifest;
   ipc_manifest.name = base::NullableString16(
       ipc_manifest.name.string().substr(0, Manifest::kMaxIPCStringLength),
       ipc_manifest.name.is_null());
   ipc_manifest.short_name = base::NullableString16(
         ipc_manifest.short_name.string().substr(0,
                                                 Manifest::kMaxIPCStringLength),
         ipc_manifest.short_name.is_null());
   for (auto& icon : ipc_manifest.icons) {
@@ skipping 11 matching lines @@
                                    0, Manifest::kMaxIPCStringLength),
                                related_application.id.is_null());
   }
 
   Send(new ManifestManagerHostMsg_RequestManifestResponse(
       routing_id(), request_id, ipc_manifest));
 }
 
 void ManifestManager::GetManifest(const GetManifestCallback& callback) {
   if (!may_have_manifest_) {
-    callback.Run(Manifest());
+    callback.Run(Manifest(), ManifestDebugInfo());
     return;
   }
 
   if (!manifest_dirty_) {
-    callback.Run(manifest_);
+    callback.Run(manifest_, manifest_debug_info_);
     return;
   }
 
   pending_callbacks_.push_back(callback);
 
   // Just wait for the running call to be done if there are other callbacks.
   if (pending_callbacks_.size() > 1)
     return;
 
   FetchManifest();
@@ skipping 36 matching lines @@
     const GURL& document_url,
     const blink::WebURLResponse& response,
     const std::string& data) {
   if (response.isNull() && data.empty()) {
     ManifestUmaUtil::FetchFailed(ManifestUmaUtil::FETCH_UNSPECIFIED_REASON);
     ResolveCallbacks(ResolveStateFailure);
     return;
   }
 
   ManifestUmaUtil::FetchSucceeded();
-
   ManifestParser parser(data, response.url(), document_url);
   parser.Parse();
 
   fetcher_.reset();
+  manifest_debug_info_.raw_data = data;
+  parser.TakeErrors(&manifest_debug_info_.errors);
 
-  for (const std::unique_ptr<ManifestParser::ErrorInfo>& error_info :
-       parser.errors()) {
+  for (const auto& error : manifest_debug_info_.errors) {
     blink::WebConsoleMessage message;
     message.level = blink::WebConsoleMessage::LevelError;
-    message.text = blink::WebString::fromUTF8(error_info->error_msg);
+    message.text = blink::WebString::fromUTF8(error.message);
     message.url =
         render_frame()->GetWebFrame()->document().manifestURL().string();
-    message.lineNumber = error_info->error_line;
-    message.columnNumber = error_info->error_column;
+    message.lineNumber = error.line;
+    message.columnNumber = error.column;
     render_frame()->GetWebFrame()->addMessageToConsole(message);
   }
 
   // Having errors while parsing the manifest doesn't mean the manifest parsing
   // failed. Some properties might have been ignored but some others kept.
   if (parser.failed()) {
     ResolveCallbacks(ResolveStateFailure);
     return;
   }
 
   manifest_ = parser.manifest();
   ResolveCallbacks(ResolveStateSuccess);
 }
 
 void ManifestManager::ResolveCallbacks(ResolveState state) {
   if (state == ResolveStateFailure)
     manifest_ = Manifest();
 
   manifest_dirty_ = state != ResolveStateSuccess;
 
-  Manifest manifest = manifest_;

[dgozman, 2016/04/29 02:28:26]

Maybe |manifest_| could change during callback, and so it was saved to a var?

[pfeldman, 2016/04/29 02:36:18]

Originally, I did not see how it was possible, but now I can see how existing
code is racy. If navigation commits between request and response,
ResolveCallbacks would overwrite manifest_dirty_ and we would end up with the
old manifest, etc.

-  std::list<GetManifestCallback> callbacks = pending_callbacks_;
-
-  pending_callbacks_.clear();
+  std::list<GetManifestCallback> callbacks;
+  callbacks.swap(pending_callbacks_);
 
   for (std::list<GetManifestCallback>::const_iterator it = callbacks.begin();
        it != callbacks.end(); ++it) {
-    it->Run(manifest);
+    it->Run(manifest_, manifest_debug_info_);
   }
 }
 
 } // namespace content