[ios] Add URL origin to synthetic crash reports

Notable changes:
 - Record pending URL for chrome://inducebrowsercrashforrealz
   to distinguish self inflicted UTEs
 - Extend PreviousSessionInfo to store navigation URLs
 - PreviousSessionInfo only stores URL origins as requested
   by Privacy team
 - Use URLs from PreviousSessionInfo for synthetic crash reports

Bug: 1103752
Change-Id: Idbde05b634e840fe377c4803b357fbef03c36170
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2451298
Commit-Queue: Eugene But <eugenebut@chromium.org>
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Auto-Submit: Eugene But <eugenebut@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814360}

ios/chrome/browser/crash_report/BUILD.gn

@@ -70,6 +70,7 @@ source_set("crash_report_internal") {
     "//ios/chrome/browser/infobars",
     "//ios/chrome/browser/infobars:public",
     "//ios/chrome/browser/main:public",
+    "//ios/chrome/browser/metrics:previous_session_info",
     "//ios/chrome/browser/sessions",
     "//ios/chrome/browser/sessions:restoration_agent",
     "//ios/chrome/browser/sessions:serialisation",

ios/chrome/browser/crash_report/crash_reporter_url_observer.h

@@ -11,6 +11,7 @@
 
 #include "ios/chrome/browser/web_state_list/web_state_list_observer.h"
 #include "ios/web/public/web_state_observer.h"
+#include "url/gurl.h"
 
 namespace web {
 class NavigationContext;
@@ -21,7 +22,7 @@ class AllWebStateObservationForwarder;
 // Provides method to set and remove parameter values.
 @protocol CrashReporterParameterSetter
 // Sets a parameter named |key| with value |value|.
-- (void)setReportParameterValue:(NSString*)value forKey:(NSString*)key;
+- (void)setReportParameterURL:(const GURL&)URL forKey:(NSString*)key;
 
 // Deletes the parameter |key|.
 - (void)removeReportParameter:(NSString*)key;
@@ -43,7 +44,7 @@ class CrashReporterURLObserver : public WebStateListObserver,
   // Records the given URL associated to the given id to the list of URLs to
   // send to the crash server. If |pending| is true, the URL is one that is
   // expected to start loading, but hasn't actually been seen yet.
-  void RecordURL(NSString* url, web::WebState* web_state, bool pending);
+  void RecordURL(const GURL& url, web::WebState* web_state, bool pending);
 
   // Observes |webState| by this instance of the CrashReporterURLObserver.
   void ObservePreloadWebState(web::WebState* web_state);

ios/chrome/browser/crash_report/crash_reporter_url_observer.mm

@@ -12,6 +12,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
 #include "ios/chrome/browser/crash_report/breakpad_helper.h"
+#import "ios/chrome/browser/metrics/previous_session_info.h"
 #import "ios/chrome/browser/web_state_list/all_web_state_observation_forwarder.h"
 #import "ios/chrome/browser/web_state_list/web_state_list.h"
 #include "ios/web/public/browser_state.h"
@@ -41,16 +42,21 @@ const char kPreloadWebStateGroup[] = "PreloadGroup";
 
 }  // namespace
 
-// A CrashReporterParameterSetter that forward parameters to Breakpad.
-@interface BreakpadParamSetter : NSObject <CrashReporterParameterSetter>
+// A CrashReporterParameterSetter that forward parameters to Breakpad and
+// PreviousSessionInfo.
+@interface CrashReporterParameterSetter
+    : NSObject <CrashReporterParameterSetter>
 @end
 
-@implementation BreakpadParamSetter
+@implementation CrashReporterParameterSetter
 - (void)removeReportParameter:(NSString*)key {
   breakpad_helper::RemoveReportParameter(key);
+  [[PreviousSessionInfo sharedInstance] removeReportParameterForKey:key];
 }
-- (void)setReportParameterValue:(NSString*)value forKey:(NSString*)key {
-  breakpad_helper::AddReportParameter(key, value, true);
+- (void)setReportParameterURL:(const GURL&)URL forKey:(NSString*)key {
+  breakpad_helper::AddReportParameter(key, base::SysUTF8ToNSString(URL.spec()),
+                                      true);
+  [[PreviousSessionInfo sharedInstance] setReportParameterURL:URL forKey:key];
 }
 @end
 
@@ -59,7 +65,7 @@ const char kPreloadWebStateGroup[] = "PreloadGroup";
 // static
 CrashReporterURLObserver* CrashReporterURLObserver::GetSharedInstance() {
   static CrashReporterURLObserver* instance =
-      new CrashReporterURLObserver([[BreakpadParamSetter alloc] init]);
+      new CrashReporterURLObserver([[CrashReporterParameterSetter alloc] init]);
   return instance;
 }
 
@@ -103,7 +109,7 @@ void CrashReporterURLObserver::RemoveWebStateList(
 
 #pragma mark - Record URLs
 
-void CrashReporterURLObserver::RecordURL(NSString* url,
+void CrashReporterURLObserver::RecordURL(const GURL& url,
                                          web::WebState* web_state,
                                          bool pending) {
   DCHECK(!web_state->GetBrowserState()->IsOffTheRecord());
@@ -134,9 +140,9 @@ void CrashReporterURLObserver::RecordURL(NSString* url,
   if (pending) {
     if (reusing_key)
       [params_setter_ removeReportParameter:breakpad_key];
-    [params_setter_ setReportParameterValue:url forKey:pending_key];
+    [params_setter_ setReportParameterURL:url forKey:pending_key];
   } else {
-    [params_setter_ setReportParameterValue:url forKey:breakpad_key];
+    [params_setter_ setReportParameterURL:url forKey:breakpad_key];
     [params_setter_ removeReportParameter:pending_key];
   }
 }
@@ -146,8 +152,7 @@ void CrashReporterURLObserver::RecordURLForWebState(web::WebState* web_state) {
       web_state->GetNavigationManager()->GetPendingItem();
   const GURL& url =
       pending_item ? pending_item->GetURL() : web_state->GetLastCommittedURL();
-  RecordURL(base::SysUTF8ToNSString(url.spec()), web_state,
-            pending_item != nullptr);
+  RecordURL(url, web_state, pending_item != nullptr);
 }
 
 #pragma mark - Start/Stop observing
@@ -256,20 +261,19 @@ void CrashReporterURLObserver::WebStateActivatedAt(
 void CrashReporterURLObserver::DidStartNavigation(
     web::WebState* web_state,
     web::NavigationContext* navigation_context) {
-  NSString* url_string =
-      base::SysUTF8ToNSString(navigation_context->GetUrl().spec());
-  if (!url_string.length || web_state->GetBrowserState()->IsOffTheRecord())
+  if (navigation_context->GetUrl().spec().empty() ||
+      web_state->GetBrowserState()->IsOffTheRecord()) {
     return;
-  RecordURL(url_string, web_state, true);
+  }
+  RecordURL(navigation_context->GetUrl(), web_state, true);
 }
 
 void CrashReporterURLObserver::DidFinishNavigation(
     web::WebState* web_state,
     web::NavigationContext* navigation_context) {
-  NSString* url_string =
-      base::SysUTF8ToNSString(navigation_context->GetUrl().spec());
-
-  if (!url_string.length || web_state->GetBrowserState()->IsOffTheRecord())
+  if (navigation_context->GetUrl().spec().empty() ||
+      web_state->GetBrowserState()->IsOffTheRecord()) {
     return;
-  RecordURL(url_string, web_state, false);
+  }
+  RecordURL(navigation_context->GetUrl(), web_state, false);
 }

ios/chrome/browser/crash_report/crash_reporter_url_observer_unittest.mm

@@ -6,6 +6,7 @@
 
 #include "ios/chrome/browser/crash_report/crash_reporter_url_observer.h"
 
+#include "base/strings/sys_string_conversions.h"
 #include "base/test/task_environment.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
 #import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h"
@@ -75,8 +76,8 @@ class TestWebState : public web::TestWebState {
   [_params removeObjectForKey:key];
 }
 
-- (void)setReportParameterValue:(NSString*)value forKey:(NSString*)key {
-  [_params setObject:value forKey:key];
+- (void)setReportParameterURL:(const GURL&)URL forKey:(NSString*)key {
+  [_params setObject:base::SysUTF8ToNSString(URL.spec()) forKey:key];
 }
 
 @end

ios/chrome/browser/crash_report/synthetic_crash_report_util.mm

@@ -93,6 +93,14 @@ void CreateSyntheticCrashReportForUte(
   AppendConfig(config, "BreakpadServerParameterPrefix_platform",
                base::SysInfo::HardwareModelName());
 
+  for (NSString* key in previous_session.reportParameterURLs.allKeys) {
+    AppendConfig(
+        config,
+        base::StringPrintf("BreakpadServerParameterPrefix_%s",
+                           base::SysNSStringToUTF8(key).c_str()),
+        base::SysNSStringToUTF8(previous_session.reportParameterURLs[key]));
+  }
+
   // Write empty minidump file, as Breakpad can't upload config without the
   // minidump.
   base::File minidump_file(

ios/chrome/browser/crash_report/synthetic_crash_report_util_unittest.mm

@@ -40,6 +40,8 @@ TEST_F(SyntheticCrashReportUtilTest, CreateSyntheticCrashReportForUte) {
   NSString* const kOSVersion = @"OSVersion";
   previous_session.OSVersion = kOSVersion;
   previous_session.terminatedDuringSessionRestoration = YES;
+  NSString* const kURL = @"URL";
+  previous_session.reportParameterURLs = @{@"url" : kURL};
 
   // Create crash report.
   base::ScopedTempDir temp_dir;
@@ -96,7 +98,7 @@ TEST_F(SyntheticCrashReportUtilTest, CreateSyntheticCrashReportForUte) {
 
   // Verify config file content. Config file has the following format:
   // <Key1>\n<Value1Length>\n<Value1>\n...<KeyN>\n<ValueNLength>\n<ValueN>
-  ASSERT_EQ(39U, config_lines.size())
+  ASSERT_EQ(42U, config_lines.size())
       << "<content>" << config_content << "</content>";
 
   EXPECT_EQ("MinidumpDir", config_lines[0]);
@@ -161,6 +163,10 @@ TEST_F(SyntheticCrashReportUtilTest, CreateSyntheticCrashReportForUte) {
             config_lines[37]);
   EXPECT_EQ(base::SysInfo::HardwareModelName(), config_lines[38]);
 
+  EXPECT_EQ("BreakpadServerParameterPrefix_url", config_lines[39]);
+  EXPECT_EQ(base::NumberToString(kURL.length), config_lines[40]);
+  EXPECT_EQ(base::SysNSStringToUTF8(kURL), config_lines[41]);
+
   // Read minidump file. It must be empty as there is no stack trace, but
   // Breakpad will not upload config without minidump file.
   base::File minidump_file(minidump_file_path,

ios/chrome/browser/metrics/previous_session_info.h

@@ -9,6 +9,8 @@
 
 #include "base/callback_helpers.h"
 
+#include "url/gurl.h"
+
 namespace previous_session_info_constants {
 // Key in the UserDefaults for a boolean value keeping track of memory warnings.
 extern NSString* const kDidSeeMemoryWarningShortlyBeforeTerminating;
@@ -20,6 +22,8 @@ extern NSString* const kPreviousSessionInfoRestoringSession;
 // Key in the UserDefaults for an array which contains the ids for the connected
 // scene sessions on the previous run.
 extern NSString* const kPreviousSessionInfoConnectedSceneSessionIDs;
+// Key in the UserDefaults for a dictionary with navigation URLs.
+extern NSString* const kPreviousSessionInfoURLs;
 
 // The values of this enum are persisted (both to NSUserDefaults and logs) and
 // represent the state of the last session (which may have been running a
@@ -117,6 +121,10 @@ enum class DeviceBatteryState {
 @property(nonatomic, readonly)
     NSMutableSet<NSString*>* connectedSceneSessionsIDs;
 
+// Pending and committed navigation URLs.
+@property(nonatomic, readonly)
+    NSDictionary<NSString*, NSString*>* reportParameterURLs;
+
 // Singleton PreviousSessionInfo. During the lifetime of the app, the returned
 // object is the same, and describes the previous session, even after a new
 // session has started (by calling beginRecordingCurrentSession).
@@ -170,6 +178,10 @@ enum class DeviceBatteryState {
 // gets destructed.
 - (void)resetSessionRestorationFlag;
 
+// Records information about pending and committed navigation URLs.
+- (void)setReportParameterURL:(const GURL&)URL forKey:(NSString*)key;
+- (void)removeReportParameterForKey:(NSString*)key;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_METRICS_PREVIOUS_SESSION_INFO_H_

ios/chrome/browser/metrics/previous_session_info.mm

@@ -94,7 +94,6 @@ NSString* const kPreviousSessionInfoLowPowerMode =
 //   version of the application.
 NSString* const kPreviousSessionInfoMultiWindowEnabled =
     @"PreviousSessionInfoMultiWindowEnabled";
-
 }  // namespace
 
 namespace previous_session_info_constants {
@@ -105,6 +104,7 @@ NSString* const kPreviousSessionInfoRestoringSession =
     @"PreviousSessionInfoRestoringSession";
 NSString* const kPreviousSessionInfoConnectedSceneSessionIDs =
     @"PreviousSessionInfoConnectedSceneSessionIDs";
+NSString* const kPreviousSessionInfoURLs = @"PreviousSessionInfoURLs";
 }  // namespace previous_session_info_constants
 
 @interface PreviousSessionInfo ()
@@ -131,6 +131,8 @@ NSString* const kPreviousSessionInfoConnectedSceneSessionIDs =
 @property(nonatomic, strong) NSDate* sessionEndTime;
 @property(nonatomic, assign) BOOL terminatedDuringSessionRestoration;
 @property(nonatomic, strong) NSMutableSet<NSString*>* connectedSceneSessionsIDs;
+@property(nonatomic, copy)
+    NSDictionary<NSString*, NSString*>* reportParameterURLs;
 
 @end
 
@@ -213,6 +215,10 @@ static PreviousSessionInfo* gSharedInstance = nil;
     gSharedInstance.terminatedDuringSessionRestoration =
         [defaults boolForKey:previous_session_info_constants::
                                  kPreviousSessionInfoRestoringSession];
+
+    gSharedInstance.reportParameterURLs =
+        [defaults dictionaryForKey:previous_session_info_constants::
+                                       kPreviousSessionInfoURLs];
   }
   return gSharedInstance;
 }
@@ -469,4 +475,35 @@ static PreviousSessionInfo* gSharedInstance = nil;
   [NSUserDefaults.standardUserDefaults synchronize];
 }
 
+- (void)setReportParameterURL:(const GURL&)URL forKey:(NSString*)key {
+  NSMutableDictionary* URLs = [[NSUserDefaults.standardUserDefaults
+      dictionaryForKey:previous_session_info_constants::
+                           kPreviousSessionInfoURLs] mutableCopy];
+  if (!URLs) {
+    URLs = [NSMutableDictionary dictionaryWithCapacity:1];
+  }
+  // Store only URL origin (not whole URL spec) as requested by Privacy Team.
+  URLs[key] = base::SysUTF8ToNSString(URL.GetOrigin().spec().c_str());
+  [NSUserDefaults.standardUserDefaults
+      setObject:URLs
+         forKey:previous_session_info_constants::kPreviousSessionInfoURLs];
+  [NSUserDefaults.standardUserDefaults synchronize];
+}
+
+- (void)removeReportParameterForKey:(NSString*)key {
+  NSMutableDictionary* URLs = [[NSUserDefaults.standardUserDefaults
+      dictionaryForKey:previous_session_info_constants::
+                           kPreviousSessionInfoURLs] mutableCopy];
+  if (URLs) {
+    URLs[key] = nil;
+    if (URLs.count == 0) {
+      URLs = nil;
+    }
+    [NSUserDefaults.standardUserDefaults
+        setObject:URLs
+           forKey:previous_session_info_constants::kPreviousSessionInfoURLs];
+    [NSUserDefaults.standardUserDefaults synchronize];
+  }
+}
+
 @end

ios/chrome/browser/metrics/previous_session_info_private.h

@@ -20,6 +20,8 @@
 @property(nonatomic, copy) NSString* OSVersion;
 @property(nonatomic, assign) BOOL terminatedDuringSessionRestoration;
 @property(nonatomic, strong) NSMutableSet<NSString*>* connectedSceneSessionsIDs;
+@property(nonatomic, copy)
+    NSDictionary<NSString*, NSString*>* reportParameterURLs;
 
 + (void)resetSharedInstanceForTesting;
 

ios/chrome/browser/metrics/previous_session_info_unittest.mm

@@ -472,4 +472,84 @@ TEST_F(PreviousSessionInfoTest,
       terminatedDuringSessionRestoration]);
 }
 
+// Tests added and removing navigation URLs.
+TEST_F(PreviousSessionInfoTest, ReportParameterURLs) {
+  // Default state.
+  [NSUserDefaults.standardUserDefaults
+      removeObjectForKey:previous_session_info_constants::
+                             kPreviousSessionInfoURLs];
+
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  EXPECT_FALSE([PreviousSessionInfo sharedInstance].reportParameterURLs);
+
+  // Removing non-existing key does not crash.
+  NSString* const kKey0 = @"url0";
+  [[PreviousSessionInfo sharedInstance] removeReportParameterForKey:kKey0];
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  EXPECT_FALSE([PreviousSessionInfo sharedInstance].reportParameterURLs);
+
+  // Add first URL.
+  [[PreviousSessionInfo sharedInstance]
+      setReportParameterURL:GURL("https://example.test/path")
+                     forKey:kKey0];
+  NSDictionary<NSString*, NSString*>* URLs =
+      [NSUserDefaults.standardUserDefaults
+          dictionaryForKey:previous_session_info_constants::
+                               kPreviousSessionInfoURLs];
+  EXPECT_NSEQ(@{kKey0 : @"https://example.test/"}, URLs);  // stores only origin
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  EXPECT_NSEQ(URLs, [[PreviousSessionInfo sharedInstance] reportParameterURLs]);
+
+  // Update first URL.
+  [[PreviousSessionInfo sharedInstance]
+      setReportParameterURL:GURL("https://example2.test/path")
+                     forKey:kKey0];
+  URLs = [NSUserDefaults.standardUserDefaults
+      dictionaryForKey:previous_session_info_constants::
+                           kPreviousSessionInfoURLs];
+  EXPECT_NSEQ(@{kKey0 : @"https://example2.test/"}, URLs);
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  EXPECT_NSEQ(URLs, [[PreviousSessionInfo sharedInstance] reportParameterURLs]);
+
+  // Add second URL.
+  NSString* const kKey1 = @"url1";
+  [[PreviousSessionInfo sharedInstance]
+      setReportParameterURL:GURL("https://example3.test/path")
+                     forKey:kKey1];
+  URLs = [NSUserDefaults.standardUserDefaults
+      dictionaryForKey:previous_session_info_constants::
+                           kPreviousSessionInfoURLs];
+  NSDictionary<NSString*, NSString*>* expected = @{
+    kKey0 : @"https://example2.test/",
+    kKey1 : @"https://example3.test/",
+  };
+  EXPECT_NSEQ(expected, URLs);
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  EXPECT_NSEQ(URLs, [[PreviousSessionInfo sharedInstance] reportParameterURLs]);
+
+  // Removing non-existing key does not crash.
+  [[PreviousSessionInfo sharedInstance] removeReportParameterForKey:@"url2"];
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  EXPECT_NSEQ(URLs, [[PreviousSessionInfo sharedInstance] reportParameterURLs]);
+
+  // Remove first URL.
+  [[PreviousSessionInfo sharedInstance] removeReportParameterForKey:kKey0];
+  URLs = [NSUserDefaults.standardUserDefaults
+      dictionaryForKey:previous_session_info_constants::
+                           kPreviousSessionInfoURLs];
+  EXPECT_NSEQ(@{kKey1 : @"https://example3.test/"}, URLs);
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  EXPECT_NSEQ(URLs, [[PreviousSessionInfo sharedInstance] reportParameterURLs]);
+
+  // Remove second URL.
+  [[PreviousSessionInfo sharedInstance] removeReportParameterForKey:kKey1];
+  URLs = [NSUserDefaults.standardUserDefaults
+      dictionaryForKey:previous_session_info_constants::
+                           kPreviousSessionInfoURLs];
+  EXPECT_FALSE(URLs);
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  EXPECT_FALSE([[PreviousSessionInfo sharedInstance] reportParameterURLs]);
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+}
+
 }  // namespace

ios/chrome/browser/url_loading/BUILD.gn

@@ -31,6 +31,7 @@ source_set("url_loading") {
     "//ios/chrome/app/application_delegate:tab_opening",
     "//ios/chrome/browser",
     "//ios/chrome/browser/browser_state",
+    "//ios/chrome/browser/crash_report:crash_report_internal",
     "//ios/chrome/browser/feature_engagement",
     "//ios/chrome/browser/main:public",
     "//ios/chrome/browser/prerender",

ios/chrome/browser/url_loading/url_loading_browser_agent.mm

@@ -9,6 +9,7 @@
 #include "base/task/thread_pool.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/chrome_url_constants.h"
+#include "ios/chrome/browser/crash_report/crash_reporter_url_observer.h"
 #import "ios/chrome/browser/main/browser.h"
 #import "ios/chrome/browser/prerender/prerender_service.h"
 #import "ios/chrome/browser/prerender/prerender_service_factory.h"
@@ -158,10 +159,15 @@ void UrlLoadingBrowserAgent::LoadUrlInCurrentTab(const UrlLoadParams& params) {
 
   notifier_->TabWillLoadUrl(web_params.url, web_params.transition_type);
 
+  WebStateList* web_state_list = browser_->GetWebStateList();
+  web::WebState* current_web_state = web_state_list->GetActiveWebState();
+
   // NOTE: This check for the Crash Host URL is here to avoid the URL from
   // ending up in the history causing the app to crash at every subsequent
   // restart.
   if (web_params.url.host() == kChromeUIBrowserCrashHost) {
+    CrashReporterURLObserver::GetSharedInstance()->RecordURL(
+        web_params.url, current_web_state, /*pending=*/true);
     InduceBrowserCrash(web_params.url);
     // Under a debugger, the app can continue working even after the CHECK.
     // Adding a return avoids adding the crash url to history.
@@ -176,8 +182,6 @@ void UrlLoadingBrowserAgent::LoadUrlInCurrentTab(const UrlLoadParams& params) {
   // load a disallowed URL, instead create a new tab not in the incognito state.
   // Also if there's no current web state, that means there is no current tab
   // to open in, so this also redirects to a new tab.
-  WebStateList* web_state_list = browser_->GetWebStateList();
-  web::WebState* current_web_state = web_state_list->GetActiveWebState();
   if (!current_web_state || (browser_state->IsOffTheRecord() &&
                              !IsURLAllowedInIncognito(web_params.url))) {
     if (prerenderService) {