Issue 832879 link

Starred by 2 users

Issue metadata

Status:	Assigned
Owner:	erikc...@chromium.org
Cc:	sky@chromium.org
EstimatedDays:	----
NextAction:	----
OS:	----
Pri:	3
Type:	Bug
merge-merged-testbranch

Refactor: Create clean ownership semantics for content::WebContents

Project Member Reported by erikc...@chromium.org, Apr 13 2018

Issue description

This class is passed around as a raw pointer, and consumers make assumptions about the lifetime semantics. 

Currently, TabStripModel assumes ownership of the WebContents in the tab strip, but also listens for destruction of the WebContents, in case the WebContents is deleted out from under the TabStripModel. This leads to invocations of callbacks on observers of the TabStripModel that use a half-destroyed WebContents.

I'm going to take a stab at this to see how terrible it will be to have clean ownership semantics in all of chrome.

Project Member

Comment 1 by bugdroid1@chromium.org, Apr 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7171fc13e4ce614161cf67ae6bf67509f4879cb

commit e7171fc13e4ce614161cf67ae6bf67509f4879cb
Author: Erik Chen <erikchen@chromium.org>
Date: Mon Apr 16 21:20:31 2018

Update TabStripModel::ReplaceWebContentsAt to return a scoped object.

This CL is a refactor with no intended behavior change.

This CL also updates Browser::SwapTabContents() to return a scoped object, as
this was a caller of TabStripModel::ReplaceWebContentsAt.

The end goal is to update all instances of content::WebContents to have explicit
ownership semantics, which will allow us to clean up the internal ownership
semantics of TabStripModel.

Bug: 832879
Change-Id: I3ff525f8a265eb7a2abb0ee3921ac74b6b7e4271
Reviewed-on: https://chromium-review.googlesource.com/1012574
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551121}
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/android/tab_android.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/android/tab_android.h
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/dom_distiller/tab_utils.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/prerender/prerender_manager.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/resource_coordinator/tab_activity_watcher_unittest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/resource_coordinator/tab_lifecycle_unit_source_unittest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/sessions/session_restore_android.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/sessions/session_restore_browsertest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/browser.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/browser.h
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/exclusive_access/fullscreen_controller_state_unittest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tab_contents/core_tab_helper_delegate.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tab_contents/core_tab_helper_delegate.h
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tabs/tab_strip_model.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tabs/tab_strip_model.h
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tabs/tab_strip_model_stats_recorder_unittest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tabs/window_activity_watcher_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7171fc13e4ce614161cf67ae6bf67509f4879cb

commit e7171fc13e4ce614161cf67ae6bf67509f4879cb
Author: Erik Chen <erikchen@chromium.org>
Date: Mon Apr 16 21:20:31 2018

Update TabStripModel::ReplaceWebContentsAt to return a scoped object.

This CL is a refactor with no intended behavior change.

This CL also updates Browser::SwapTabContents() to return a scoped object, as
this was a caller of TabStripModel::ReplaceWebContentsAt.

The end goal is to update all instances of content::WebContents to have explicit
ownership semantics, which will allow us to clean up the internal ownership
semantics of TabStripModel.

Bug: 832879
Change-Id: I3ff525f8a265eb7a2abb0ee3921ac74b6b7e4271
Reviewed-on: https://chromium-review.googlesource.com/1012574
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551121}
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/android/tab_android.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/android/tab_android.h
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/dom_distiller/tab_utils.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/prerender/prerender_manager.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/resource_coordinator/tab_activity_watcher_unittest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/resource_coordinator/tab_lifecycle_unit_source_unittest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/sessions/session_restore_android.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/sessions/session_restore_browsertest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/browser.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/browser.h
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/exclusive_access/fullscreen_controller_state_unittest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tab_contents/core_tab_helper_delegate.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tab_contents/core_tab_helper_delegate.h
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tabs/tab_strip_model.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tabs/tab_strip_model.h
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tabs/tab_strip_model_stats_recorder_unittest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
[modify] https://crrev.com/e7171fc13e4ce614161cf67ae6bf67509f4879cb/chrome/browser/ui/tabs/window_activity_watcher_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c81fcb20769de505f681ca6d68a10b516da38a0

commit 8c81fcb20769de505f681ca6d68a10b516da38a0
Author: Erik Chen <erikchen@chromium.org>
Date: Fri Apr 20 23:12:13 2018

[Reland #1] Update ownership semantics for TabStripModel::DetachWebContentsAt.

The initial CL had a logic bug in browser_window_controller.mm that caused
crashed on macOS.

> This CL is a refactor with no intended behavior change.
>
> Previously, the method returned a raw pointer, and ownership was implicitly
> passed. This CL updates the method to returned a unique_ptr. In this process of
> updating all call sites, this CL also:
>   * Gets rid of UnloadControllerWebContentsDelegate, which added an unnecessary
>     layer of abstraction. FastUnloadController can directly inherit from
>     content::WebContentsDelegate, which removes an extraneous class.
>   * Updates FastUnloadController to hold WebContents waiting for deletion in a
>     vector of unique_ptrs. Previously, the WebContents were simply dropped on
>     the floor, and then UnloadControllerWebContentsDelegate would delete the
>     pointer it received in a callback.
>   * Updates DevToolsWindow to use clear ownership semantics for the
>     main_web_contents_. Ownership passes between DevToolsWindow and its member
>     browser_, based on the |is_docked_| parameter, but ownership was not being
>     explicitly specified.
>
> Change-Id: I4c21f04fe5d60a5149d79bda7b6ba3535bef2b7f
> Bug: 832879
> Reviewed-on: https://chromium-review.googlesource.com/1012938
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#552063}

Change-Id: Ic2af1f7a8c02e923eda3cfd980d0b845d619f00d
Bug: 832879
TBR: sky@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1022750
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552514}
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/devtools/devtools_window.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/devtools/devtools_window.h
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/extensions/api/tabs/tabs_api.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/extensions/bookmark_app_navigation_throttle_utils.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/media/webrtc/tab_desktop_media_list_unittest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/permissions/permission_request_manager_browsertest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/resource_coordinator/tab_activity_watcher_browsertest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/resource_coordinator/tab_lifecycle_unit_source_unittest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/sync/test/integration/sessions_helper.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/browser.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/exclusive_access/fullscreen_controller_state_unittest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/extensions/application_launch.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/fast_unload_controller.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/fast_unload_controller.h
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/startup/session_crashed_infobar_delegate_unittest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/tabs/tab_strip_model.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/tabs/tab_strip_model.h
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/tabs/tab_strip_model_stats_recorder_unittest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/tabs/window_activity_watcher_unittest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/unload_controller.h
[delete] https://crrev.com/30b664a36b2876ce4684ab583ee1eaa00991118d/chrome/browser/ui/unload_controller_web_contents_delegate.cc
[delete] https://crrev.com/30b664a36b2876ce4684ab583ee1eaa00991118d/chrome/browser/ui/unload_controller_web_contents_delegate.h
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/views/constrained_window_views_browsertest.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/8c81fcb20769de505f681ca6d68a10b516da38a0/chrome/browser/ui/views/tabs/tab_drag_controller.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc48c25ef9188147c14cae55c690be18ec2a29f6

commit dc48c25ef9188147c14cae55c690be18ec2a29f6
Author: erikchen <erikchen@chromium.org>
Date: Tue Apr 24 00:31:45 2018

Update TabStripModel::InsertWebContentsAt to use ownership semantics.

This CL is a refactor with no intended behavior change.

Previously, the method took a raw pointer. Now, the method takes a
std::unique_ptr.

Change-Id: I20135ca41335facbf3dd734445804daa314fe9fe
Bug: 832879
Reviewed-on: https://chromium-review.googlesource.com/1022291
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552923}
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/extensions/api/tabs/tabs_api.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/permissions/permission_request_manager_browsertest.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/resource_coordinator/tab_activity_watcher_browsertest.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ssl/security_state_tab_helper_browsertest.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/sync/test/integration/sessions_helper.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/browser_tab_strip_model_delegate.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/browser_tabrestore.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/exclusive_access/fullscreen_controller_state_unittest.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/tabs/tab_strip_model.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/tabs/tab_strip_model.h
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/tabs/tab_strip_model_stats_recorder_unittest.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/tabs/window_activity_watcher_unittest.cc
[modify] https://crrev.com/dc48c25ef9188147c14cae55c690be18ec2a29f6/chrome/browser/ui/views/tabs/tab_drag_controller.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b66dce51fef1df008a0c608707e06900f367a490

commit b66dce51fef1df008a0c608707e06900f367a490
Author: erikchen <erikchen@chromium.org>
Date: Tue Apr 24 20:30:23 2018

Update ownership semantics for TabStripModel::AppendWebContents

This CL is a refactor with no intended behavior change.

This CL updates TabStripModel::AppendWebContents to take a std::unique_ptr
rather than a raw pointer.

Change-Id: Ie04a8bc9299108d65449a765e5cffaebf9ca91ff
Bug: 832879
Reviewed-on: https://chromium-review.googlesource.com/1024296
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553245}
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/dom_distiller/tab_utils_browsertest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/extensions/api/declarative_content/declarative_content_apitest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/extensions/api/tabs/tabs_api_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/extensions/bookmark_app_navigation_throttle_utils.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/extensions/extension_context_menu_model_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/media/webrtc/tab_desktop_media_list_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/net/websocket_browsertest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/password_manager/password_manager_test_base.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/profile_resetter/profile_resetter_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/resource_coordinator/tab_lifecycle_unit_source_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/resource_coordinator/tab_lifecycle_unit_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/resource_coordinator/tab_manager_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/site_per_process_interactive_browsertest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ssl/security_state_tab_helper_browsertest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/bookmarks/bookmark_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/browser_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller_unittest.mm
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/cocoa/tabs/tab_strip_controller_unittest.mm
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/cocoa/translate/translate_bubble_controller_unittest.mm
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/content_settings/content_setting_image_model_browsertest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/extensions/application_launch.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/startup/session_crashed_infobar_delegate_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/tabs/tab_activity_simulator.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/tabs/tab_strip_model.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/tabs/tab_strip_model.h
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/views/constrained_window_views_browsertest.cc
[modify] https://crrev.com/b66dce51fef1df008a0c608707e06900f367a490/chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe

commit ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe
Author: erikchen <erikchen@chromium.org>
Date: Tue Apr 24 21:19:06 2018

Update ownership semantics for TabStripModel::ReplaceWebContentsAt.

This CL is a refactor with no intended behavior change.

It updates the semantics to explicitly take ownership of the WebContents.

Bug: 832879
Change-Id: Icb8ffbd89b3d598ccdf228ad7d0e297b9f6f5bbc
Reviewed-on: https://chromium-review.googlesource.com/1026270
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553281}
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/android/tab_android.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/android/tab_android.h
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/dom_distiller/tab_utils.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/prerender/prerender_manager.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/prerender/prerender_manager.h
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/resource_coordinator/tab_activity_watcher_unittest.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/resource_coordinator/tab_lifecycle_unit_source_unittest.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/sessions/session_restore_android.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/sessions/session_restore_browsertest.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/ui/browser.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/ui/browser.h
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/ui/exclusive_access/fullscreen_controller_state_unittest.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/ui/tab_contents/core_tab_helper_delegate.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/ui/tab_contents/core_tab_helper_delegate.h
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/ui/tabs/tab_strip_model.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/ui/tabs/tab_strip_model.h
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/ui/tabs/tab_strip_model_stats_recorder_unittest.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
[modify] https://crrev.com/ecc0dcc1b1acf2e9d01679f4fbf87cef7ce379fe/chrome/browser/ui/tabs/window_activity_watcher_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/07ff21cad40372e89b1dc53ff8525f291755d260

commit 07ff21cad40372e89b1dc53ff8525f291755d260
Author: Erik Chen <erikchen@chromium.org>
Date: Tue Apr 24 22:21:22 2018

Update ownership semantics of TabStripModel::AddWebContents.

This CL is a refactor with no intended behavior change.

It updates the semantics to explicitly take ownership of the WebContents.

Bug: 832879
Change-Id: I62a6945aada916ff9538679934b2b767ea3adeb3
Reviewed-on: https://chromium-review.googlesource.com/1026311
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553317}
[modify] https://crrev.com/07ff21cad40372e89b1dc53ff8525f291755d260/chrome/browser/devtools/devtools_window.cc
[modify] https://crrev.com/07ff21cad40372e89b1dc53ff8525f291755d260/chrome/browser/password_manager/password_manager_browsertest.cc
[modify] https://crrev.com/07ff21cad40372e89b1dc53ff8525f291755d260/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/07ff21cad40372e89b1dc53ff8525f291755d260/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/07ff21cad40372e89b1dc53ff8525f291755d260/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/07ff21cad40372e89b1dc53ff8525f291755d260/chrome/browser/ui/tabs/tab_strip_model.cc
[modify] https://crrev.com/07ff21cad40372e89b1dc53ff8525f291755d260/chrome/browser/ui/tabs/tab_strip_model.h
[modify] https://crrev.com/07ff21cad40372e89b1dc53ff8525f291755d260/chrome/browser/ui/tabs/tab_strip_model_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38fa402658a53dff0ce85443b410c306fee67995

commit 38fa402658a53dff0ce85443b410c306fee67995
Author: erikchen <erikchen@chromium.org>
Date: Thu Apr 26 20:37:52 2018

Clean up ownership semantics for NavigateParams.

This CL is a refactor with no intended behavior change.

Previously, NavigateParams was a copy-able class that sometimes passed
ownership of its member target_contents to the Navigate() method. Furthermore,
target_contents was overloaded to have different semantics before, during, and
after the call to Navigate().

This CL makes NavigateParams a move-only class. It has the input parameter
|contents_to_insert| which explicitly passes ownership [with clear semantics] to
the Navigate() method. It has another input parameter |switch_to_singleton_tab|
to indicate that a tab switch should occur, but no ownership is passed. Finaly,
it has an output parameter |navigated_or_inserted_contents| which does not
convey ownership.

With these new semantics, the helper class ScopedTargetContentsOwner is no
longer necessary. Furthermore, the Navigate() method now uses an internal,
temporary variable |contents_to_navigate_or_insert| rather than overloading the
semantics of NavigateParams.target_contents.

This CL creates a new struct PrerenderManager::Params to function as both input
and output parameters to PrerenderManager::MaybeUsePrerenderedPage, rather than
overloading the NavigateParam struct to have different semantics.

Change-Id: Ic94284b9f6d2a0305375127858020271f7a97601
Bug: 832879
Reviewed-on: https://chromium-review.googlesource.com/1028458
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Sergey Volk <servolk@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554128}
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/android/tab_android.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/android/tab_web_contents_delegate_android.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/devtools/chrome_devtools_manager_delegate.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/devtools/protocol/target_handler.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/extensions/chrome_extension_host_delegate.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/extensions/chrome_extension_host_delegate.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/extensions/extension_tab_util.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/extensions/extension_tab_util.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/media/media_engagement_browsertest.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/prerender/prerender_manager.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/prerender/prerender_manager.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ssl/ssl_browsertest.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/sync/test/integration/sessions_helper.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/apps/chrome_app_delegate.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/browser.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/browser_navigator_browsertest.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/browser_navigator_params.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/browser_navigator_params.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/browser_tabstrip.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/chrome_pages.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/cocoa/applescript/window_applescript.mm
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/extensions/application_launch.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/extensions/extension_install_ui_default.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/signin_view_controller.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/singleton_tabs.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/singleton_tabs.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/views/bookmarks/bookmark_bubble_sign_in_delegate_browsertest.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/webui/chrome_web_contents_handler.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/webui/chrome_web_contents_handler.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/browser/ui/webui/inspect_ui.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/test/base/browser_with_test_window_test.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/test/base/in_process_browser_test.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chrome/test/base/ui_test_utils.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chromecast/browser/extensions/cast_extension_host_delegate.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/chromecast/browser/extensions/cast_extension_host_delegate.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/extensions/browser/extension_host.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/extensions/browser/extension_host_delegate.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/extensions/shell/browser/shell_extension_host_delegate.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/extensions/shell/browser/shell_extension_host_delegate.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/ui/web_dialogs/test/test_web_contents_handler.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/ui/web_dialogs/test/test_web_contents_handler.h
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/ui/web_dialogs/web_dialog_web_contents_delegate.cc
[modify] https://crrev.com/38fa402658a53dff0ce85443b410c306fee67995/ui/web_dialogs/web_dialog_web_contents_delegate.h

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bee5c962dce0265e3f5b78652961b56879d5045d

commit bee5c962dce0265e3f5b78652961b56879d5045d
Author: erikchen <erikchen@chromium.org>
Date: Fri Apr 27 19:30:25 2018

Update ownership semantics for WebContentsDelegate::AddWebContents.

This CL is a refactor and has no intended behavior change.

This CL updates AddWebContents to use explicit ownership semantics. This CL
fixes some of the subclasses to have proper ownership semantics, and leaves
behind some TODOs for other subclasses.

Bug: 832879, 674318
Change-Id: I6294e591e9166dfb0c029eb78648c17216075dab
TBR: skyostil@chromium.org, carlosk@chromium.org, torne@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1028631
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554450}
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/android_webview/browser/aw_web_contents_delegate.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/android_webview/browser/aw_web_contents_delegate.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/android/document/document_web_contents_delegate.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/android/document/document_web_contents_delegate.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/android/tab_web_contents_delegate_android.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/android/tab_web_contents_delegate_android.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/background/background_contents.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/background/background_contents.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/background/background_contents_service.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/background/background_contents_service.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/devtools/devtools_window.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/devtools/devtools_window.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/ssl/ssl_browsertest.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/ui/apps/chrome_app_delegate.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/ui/apps/chrome_app_delegate.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/ui/browser.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/ui/browser.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/ui/browser_tabstrip.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/ui/browser_tabstrip.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac_browsertest.mm
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/chrome/browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/components/offline_pages/content/background_loader/background_loader_contents.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/components/offline_pages/content/background_loader/background_loader_contents.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/components/offline_pages/content/background_loader/background_loader_contents_unittest.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/content/browser/web_contents/web_contents_impl_browsertest.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/content/public/browser/web_contents_delegate.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/content/shell/browser/shell.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/content/shell/browser/shell.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/content/test/test_web_contents.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/content/test/test_web_contents.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/browser/app_window/app_delegate.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/browser/app_window/app_window.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/browser/app_window/app_window.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/browser/extension_host.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/browser/extension_host.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/browser/guest_view/extension_options/extension_options_guest.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/browser/guest_view/extension_options/extension_options_guest.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/browser/guest_view/web_view/web_view_guest.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/browser/guest_view/web_view/web_view_guest.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/shell/browser/shell_app_delegate.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/extensions/shell/browser/shell_app_delegate.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/headless/lib/browser/headless_web_contents_impl.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/ui/views/controls/webview/web_dialog_view.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/ui/views/controls/webview/web_dialog_view.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/ui/web_dialogs/web_dialog_delegate.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/ui/web_dialogs/web_dialog_delegate.h
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/ui/web_dialogs/web_dialog_web_contents_delegate.cc
[modify] https://crrev.com/bee5c962dce0265e3f5b78652961b56879d5045d/ui/web_dialogs/web_dialog_web_contents_delegate.h

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b

commit bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b
Author: Erik Chen <erikchen@chromium.org>
Date: Sat Apr 28 14:10:43 2018

Make WebContents::Create() return a unique_ptr.

This CL is a refactor with no intended behavior change.

Most callers have already been fixed to accept a unique_ptr. Guest views and
headless are notable exceptions - this CL leaves TODOs to fix them up
afterwards.

This CL also fixes a layering issue in ViewsDelegate, which was incorrectly and
unnecessarily referencing contents/.

Bug: 832879
Change-Id: Ie00387ca6c182390b1d720a60deaebe8d770aa17
TBR: sky@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1031314
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554649}
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/ash/BUILD.gn
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/ash/content/display/DEPS
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/ash/content/display/screen_orientation_controller_chromeos_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/ash/test/ash_test_environment_content.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/ash/test/ash_test_environment_content.h
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/android/web_contents_factory.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/background/background_contents.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/chromeos/assistant/assistant_card_renderer.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/chromeos/first_run/drive_first_run_controller.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/devtools/devtools_window.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/dom_distiller/tab_utils.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/dom_distiller/tab_utils_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/extensions/api/declarative_content/declarative_content_apitest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/extensions/api/tab_capture/offscreen_tab.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/net/websocket_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/offline_pages/offline_page_request_job_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/offline_pages/offline_page_utils_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/password_manager/password_manager_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/password_manager/password_manager_test_base.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/profile_resetter/profile_resetter_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/sessions/session_restore_android.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/site_per_process_interactive_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ssl/security_state_tab_helper_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ssl/ssl_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/android/tab_model/android_live_tab_context.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/ash/chrome_keyboard_ui.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/ash/chrome_keyboard_ui.h
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/ash/chrome_keyboard_ui_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/bookmarks/bookmark_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/cocoa/infobars/confirm_infobar_controller_unittest.mm
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/cocoa/infobars/infobar_container_controller_unittest.mm
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/cocoa/infobars/translate_infobar_unittest.mm
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/cocoa/profiles/user_manager_mac.mm
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/cocoa/tabs/tab_strip_controller_unittest.mm
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/cocoa/translate/translate_bubble_controller_unittest.mm
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/content_settings/content_setting_image_model_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/exclusive_access/fullscreen_controller_state_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/ime/ime_window.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/webui/browsing_history_handler_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chrome/browser/ui/webui/sync_internals_message_handler_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/chromecast/browser/cast_web_view_default.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/components/dom_distiller/content/browser/distiller_page_web_contents.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/components/guest_view/browser/guest_view_manager.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/components/offline_pages/content/background_loader/background_loader_contents.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/content/browser/loader/resource_dispatcher_host_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/content/browser/site_instance_impl_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/content/browser/web_contents/web_contents_delegate_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/content/public/browser/web_contents.h
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/content/public/test/test_utils.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/content/shell/browser/shell.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/extensions/browser/api_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/extensions/browser/app_window/app_window_contents.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/extensions/browser/extension_host.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/extensions/browser/guest_view/app_view/app_view_guest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/extensions/browser/guest_view/extension_options/extension_options_guest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/extensions/browser/guest_view/extension_view/extension_view_guest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/extensions/browser/guest_view/web_view/web_view_guest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/headless/lib/browser/headless_web_contents_impl.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/ui/views/controls/webview/webview.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/ui/views/controls/webview/webview.h
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/ui/views/controls/webview/webview_unittest.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/ui/views/views_delegate.cc
[modify] https://crrev.com/bb8e738e5eb7ab4286d69494b3b89aebc3eedb3b/ui/views/views_delegate.h

Comment 11 by erikc...@chromium.org, Apr 30 2018

Mini design doc that describes the upcoming refactor for guest views: https://docs.google.com/document/d/1Sx0HOp0bm860sUeDfpp8L7txAwUFK6QKln8UMdUbfpE/edit#

Project Member

Comment 12 by bugdroid1@chromium.org, May 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59c3cd6e0244315d03c94d8189db2d81556f1363

commit 59c3cd6e0244315d03c94d8189db2d81556f1363
Author: erikchen <erikchen@chromium.org>
Date: Tue May 01 15:17:17 2018

Add DCHECK to ExtensionOptionsGuest::AddNewContents.

The DCHECK confirms that the WebContents being passed to the embedder web
content's delegate is never a guest Web Contents.

The code path that would cause that to happen [WebContentsImpl::CreateNewWindow]
early-outs because ExtensionOptionsGuest::ShouldCreateWebContents always returns
false.

Bug: 832879
Change-Id: I4e4055b2c1213f114a3e402bf353e66dfcfc6472
Reviewed-on: https://chromium-review.googlesource.com/1036244
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555042}
[modify] https://crrev.com/59c3cd6e0244315d03c94d8189db2d81556f1363/extensions/browser/guest_view/extension_options/extension_options_guest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, May 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ade1fefb8da3faf879b53598a5f61da8e4e9c72f

commit ade1fefb8da3faf879b53598a5f61da8e4e9c72f
Author: erikchen <erikchen@chromium.org>
Date: Wed May 02 22:07:57 2018

Change WebContents::CreateWithSessionStorage to use strong ownership.

This CL is a refactor with no intended behavior change.

Bug: 832879
Change-Id: I74ae0bf9b2e3cc7775025767999ffd4a661d4082
Reviewed-on: https://chromium-review.googlesource.com/1040470
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555569}
[modify] https://crrev.com/ade1fefb8da3faf879b53598a5f61da8e4e9c72f/chrome/browser/background/background_contents.cc
[modify] https://crrev.com/ade1fefb8da3faf879b53598a5f61da8e4e9c72f/chrome/browser/prerender/prerender_contents.cc
[modify] https://crrev.com/ade1fefb8da3faf879b53598a5f61da8e4e9c72f/chrome/browser/prerender/prerender_contents.h
[modify] https://crrev.com/ade1fefb8da3faf879b53598a5f61da8e4e9c72f/chrome/browser/ui/browser_tabrestore.cc
[modify] https://crrev.com/ade1fefb8da3faf879b53598a5f61da8e4e9c72f/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/ade1fefb8da3faf879b53598a5f61da8e4e9c72f/content/public/browser/web_contents.h

Project Member

Comment 14 by bugdroid1@chromium.org, May 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/361a3c45ee02018edf8bb654368460014f7a6bc8

commit 361a3c45ee02018edf8bb654368460014f7a6bc8
Author: erikchen <erikchen@chromium.org>
Date: Wed May 02 22:42:17 2018

Update HeadlessWebContentsImpl to use strong ownership semantics for WebContents.

This CL is a refactor with no intended behavior change.

Bug: 832879
Change-Id: Ic90818e6ce6fff15b323aefbe7e0150cf9bb9f8c
Reviewed-on: https://chromium-review.googlesource.com/1040474
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555585}
[modify] https://crrev.com/361a3c45ee02018edf8bb654368460014f7a6bc8/headless/lib/browser/headless_web_contents_impl.cc
[modify] https://crrev.com/361a3c45ee02018edf8bb654368460014f7a6bc8/headless/lib/browser/headless_web_contents_impl.h

Project Member

Comment 15 by bugdroid1@chromium.org, May 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6c7df7f7a0c02d7050b52f27c10a820ba92a7c5a

commit 6c7df7f7a0c02d7050b52f27c10a820ba92a7c5a
Author: erikchen <erikchen@chromium.org>
Date: Thu May 03 18:13:59 2018

Update WebContents::Clone() to return a unique_ptr.

This CL is a refactor with no intended behavior change.

Bug: 832879
Change-Id: I6d54051f50ed4b0c46f821872aafea1ba4b3fd32
Reviewed-on: https://chromium-review.googlesource.com/1040590
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555813}
[modify] https://crrev.com/6c7df7f7a0c02d7050b52f27c10a820ba92a7c5a/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/6c7df7f7a0c02d7050b52f27c10a820ba92a7c5a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/6c7df7f7a0c02d7050b52f27c10a820ba92a7c5a/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/6c7df7f7a0c02d7050b52f27c10a820ba92a7c5a/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/6c7df7f7a0c02d7050b52f27c10a820ba92a7c5a/content/public/browser/web_contents.h
[modify] https://crrev.com/6c7df7f7a0c02d7050b52f27c10a820ba92a7c5a/content/test/test_web_contents.cc
[modify] https://crrev.com/6c7df7f7a0c02d7050b52f27c10a820ba92a7c5a/content/test/test_web_contents.h

Project Member

Comment 16 by bugdroid1@chromium.org, May 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c243fcc8daf24f6140511f455559bf0f8cc8e7b

commit 7c243fcc8daf24f6140511f455559bf0f8cc8e7b
Author: erikchen <erikchen@chromium.org>
Date: Fri May 04 21:50:43 2018

Update ownership semantics for DevtoolWindow and the toolbar WebContents.

This CL is a refactor with no intended behavior change.

Bug: 832879
Change-Id: I5d2d04d37f03f8ccbaef28695bc29a6b24b2e089
Reviewed-on: https://chromium-review.googlesource.com/1040592
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556210}
[modify] https://crrev.com/7c243fcc8daf24f6140511f455559bf0f8cc8e7b/chrome/browser/devtools/devtools_window.cc
[modify] https://crrev.com/7c243fcc8daf24f6140511f455559bf0f8cc8e7b/chrome/browser/devtools/devtools_window.h

Project Member

Comment 17 by bugdroid1@chromium.org, May 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/03e9cecb69c89a5ee610bdd2e6e6c26d5cb2ee5f

commit 03e9cecb69c89a5ee610bdd2e6e6c26d5cb2ee5f
Author: Erik Chen <erikchen@chromium.org>
Date: Sat May 05 00:13:36 2018

Update BrowserCloseManager to use appropriate TabStripModel API.

BrowserCloseManager was deleting a WebContents owned by the TabStripModel.
Instead, it should ask the TabStripModel to release the WebContents.

Bug: 832879
Change-Id: I9f1c079e638d423d9f22e6c0be341d418e096354
Reviewed-on: https://chromium-review.googlesource.com/1044886
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556264}
[modify] https://crrev.com/03e9cecb69c89a5ee610bdd2e6e6c26d5cb2ee5f/chrome/browser/lifetime/browser_close_manager.cc

Project Member

Comment 18 by bugdroid1@chromium.org, May 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b8ad542b9c674168e2ad6c860bb8d42031d0b6b4

commit b8ad542b9c674168e2ad6c860bb8d42031d0b6b4
Author: erikchen <erikchen@chromium.org>
Date: Sat May 05 00:16:23 2018

Use unique_ptr in PrerenderManager::old_web_contents_list_.

This CL is a refactor with no intended behavior change.

Bug: 832879
Change-Id: I4c86d936907595a631fb9e603c55af90292ff724
Reviewed-on: https://chromium-review.googlesource.com/1044892
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556267}
[modify] https://crrev.com/b8ad542b9c674168e2ad6c860bb8d42031d0b6b4/chrome/browser/prerender/prerender_manager.cc
[modify] https://crrev.com/b8ad542b9c674168e2ad6c860bb8d42031d0b6b4/chrome/browser/prerender/prerender_manager.h

Project Member

Comment 19 by bugdroid1@chromium.org, May 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7d34c095c0094f5dda5d691a15dd029e41301884

commit 7d34c095c0094f5dda5d691a15dd029e41301884
Author: Erik Chen <erikchen@chromium.org>
Date: Sat May 05 00:18:01 2018

Use strong ownership for WebContents in BackgroundPrintingManager.

This CL is a refactor with no intended behavior change.

The WebContentsDestroyed observer notification is no longer necessary since we
use strong ownership semantics for WebContents everywhere, and this class is the
only one that can destroy its WebContents.

Bug: 832879
Change-Id: I22fef7e4296995f6d3f6a948c1be4bc537695cd1
Reviewed-on: https://chromium-review.googlesource.com/1044891
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556269}
[modify] https://crrev.com/7d34c095c0094f5dda5d691a15dd029e41301884/chrome/browser/printing/background_printing_manager.cc
[modify] https://crrev.com/7d34c095c0094f5dda5d691a15dd029e41301884/chrome/browser/printing/background_printing_manager.h
[modify] https://crrev.com/7d34c095c0094f5dda5d691a15dd029e41301884/chrome/browser/ui/webui/print_preview/print_preview_ui.cc

Project Member

Comment 20 by bugdroid1@chromium.org, May 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/50735fdc7a948f0ad1b44de7b122a479c4d6c987

commit 50735fdc7a948f0ad1b44de7b122a479c4d6c987
Author: erikchen <erikchen@chromium.org>
Date: Sat May 05 15:08:33 2018

Update WebContentsImpl::CreateWithOpener to return a unique_ptr.

This CL is a refactor with no intended behavior change.

Bug: 832879
Change-Id: I18a970240c9abcf38879c8b4d15aae848d2dd893
TBR: avi@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1040471
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556331}
[modify] https://crrev.com/50735fdc7a948f0ad1b44de7b122a479c4d6c987/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/50735fdc7a948f0ad1b44de7b122a479c4d6c987/content/browser/web_contents/web_contents_impl.h

Project Member

Comment 21 by bugdroid1@chromium.org, May 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5bab4996cec6afe4d81d22305a718f1aba52757c

commit 5bab4996cec6afe4d81d22305a718f1aba52757c
Author: Erik Chen <erikchen@chromium.org>
Date: Sat May 05 15:19:53 2018

Use unique_ptr in TestWebContents::Create.

This CL is a refactor with no intended behavior change.

Bug: 832879
Change-Id: Idd49d49da49f09e8fb72c1143fbb27c485eb7a14
Reviewed-on: https://chromium-review.googlesource.com/1045228
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556333}
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/ash/content/display/screen_orientation_controller_chromeos_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/ash/test/ash_test_environment_content.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/chromeos/login/signin_partition_manager_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/extensions/api/tabs/tabs_api_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/extensions/extension_protocols_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/extensions/webstore_inline_installer_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/geolocation/geolocation_permission_context_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/media/router/presentation/presentation_service_delegate_impl_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/media_galleries/media_file_system_registry_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/metrics/tab_stats_data_store_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/metrics/tab_stats_tracker_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/resource_coordinator/background_tab_navigation_throttle_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/resource_coordinator/tab_lifecycle_unit_source_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/resource_coordinator/tab_lifecycle_unit_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/resource_coordinator/tab_load_tracker_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/resource_coordinator/tab_manager_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/resource_coordinator/tab_manager_web_contents_data_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/sessions/session_restore_observer_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/sync/sessions/sync_sessions_web_contents_router_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/ui/ash/chrome_keyboard_ui_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/ui/browser_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/ui/omnibox/chrome_omnibox_navigation_observer_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/ui/passwords/password_manager_porter_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/ui/tabs/tab_strip_model_stats_recorder_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/ui/views/media_router/web_contents_display_observer_view_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/chrome/browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/components/guest_view/browser/guest_view_manager_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/components/offline_pages/content/background_loader/background_loader_contents_stub.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/browser/frame_host/render_widget_host_view_guest_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/browser/loader/resource_loader_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/browser/media/session/audio_focus_manager_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/browser/web_contents/aura/gesture_nav_simple_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/public/test/test_renderer_host.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/public/test/test_renderer_host.h
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/public/test/web_contents_tester.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/public/test/web_contents_tester.h
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/test/test_web_contents.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/test/test_web_contents.h
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/content/test/test_web_contents_factory.cc
[modify] https://crrev.com/5bab4996cec6afe4d81d22305a718f1aba52757c/ui/views/controls/webview/webview_unittest.cc

Project Member

Comment 22 by bugdroid1@chromium.org, May 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e55a74eae775f6e098ba50f58e8c2380ecf395ba

commit e55a74eae775f6e098ba50f58e8c2380ecf395ba
Author: Erik Chen <erikchen@chromium.org>
Date: Mon May 07 17:26:34 2018

Use unique_ptr in TabDragController and CreateNewStripWithContents.

This CL is a refactor with no intended behavior change.

Bug: 832879
Change-Id: I61fee37f03d16567979ac7be36b9fa1281df8d64
Reviewed-on: https://chromium-review.googlesource.com/1045205
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556482}
[modify] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/permissions/permission_request_manager_browsertest.cc
[modify] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/ui/browser_tab_strip_model_delegate.cc
[modify] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/ui/browser_tab_strip_model_delegate.h
[modify] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/ui/cocoa/browser_window_controller.mm
[add] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/ui/tabs/tab_strip_model_delegate.cc
[modify] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/ui/tabs/tab_strip_model_delegate.h
[modify] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/ui/tabs/test_tab_strip_model_delegate.cc
[modify] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/ui/tabs/test_tab_strip_model_delegate.h
[modify] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/ui/views/tabs/tab_drag_controller.cc
[modify] https://crrev.com/e55a74eae775f6e098ba50f58e8c2380ecf395ba/chrome/browser/ui/views/tabs/tab_drag_controller.h

Project Member

Comment 23 by bugdroid1@chromium.org, May 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76bdc5ac1dab16c4e21fe39b6aec26827d02beac

commit 76bdc5ac1dab16c4e21fe39b6aec26827d02beac
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Mon May 07 21:27:07 2018

Revert "Use unique_ptr in TabDragController and CreateNewStripWithContents."

This reverts commit e55a74eae775f6e098ba50f58e8c2380ecf395ba.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 556482 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2U1NWE3NGVhZTc3NWY2ZTA5OGJhNTBmNThlOGMyMzgwZWNmMzk1YmEM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20MSan%20Tests/9651

Sample Failed Step: browser_tests

Original change's description:
> Use unique_ptr in TabDragController and CreateNewStripWithContents.
> 
> This CL is a refactor with no intended behavior change.
> 
> Bug: 832879
> Change-Id: I61fee37f03d16567979ac7be36b9fa1281df8d64
> Reviewed-on: https://chromium-review.googlesource.com/1045205
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556482}

Change-Id: If7940d1afe8442ca8a35566c7c4684604f7ce794
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 832879
Reviewed-on: https://chromium-review.googlesource.com/1048425
Cr-Commit-Position: refs/heads/master@{#556572}
[modify] https://crrev.com/76bdc5ac1dab16c4e21fe39b6aec26827d02beac/chrome/browser/permissions/permission_request_manager_browsertest.cc
[modify] https://crrev.com/76bdc5ac1dab16c4e21fe39b6aec26827d02beac/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/76bdc5ac1dab16c4e21fe39b6aec26827d02beac/chrome/browser/ui/browser_tab_strip_model_delegate.cc
[modify] https://crrev.com/76bdc5ac1dab16c4e21fe39b6aec26827d02beac/chrome/browser/ui/browser_tab_strip_model_delegate.h
[modify] https://crrev.com/76bdc5ac1dab16c4e21fe39b6aec26827d02beac/chrome/browser/ui/cocoa/browser_window_controller.mm
[delete] https://crrev.com/856461f2365f7e76dea1f7a0391ccb41b3165be9/chrome/browser/ui/tabs/tab_strip_model_delegate.cc
[modify] https://crrev.com/76bdc5ac1dab16c4e21fe39b6aec26827d02beac/chrome/browser/ui/tabs/tab_strip_model_delegate.h
[modify] https://crrev.com/76bdc5ac1dab16c4e21fe39b6aec26827d02beac/chrome/browser/ui/tabs/test_tab_strip_model_delegate.cc
[modify] https://crrev.com/76bdc5ac1dab16c4e21fe39b6aec26827d02beac/chrome/browser/ui/tabs/test_tab_strip_model_delegate.h
[modify] https://crrev.com/76bdc5ac1dab16c4e21fe39b6aec26827d02beac/chrome/browser/ui/views/tabs/tab_drag_controller.cc
[modify] https://crrev.com/76bdc5ac1dab16c4e21fe39b6aec26827d02beac/chrome/browser/ui/views/tabs/tab_drag_controller.h

Project Member

Comment 24 by bugdroid1@chromium.org, May 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/07316673b145c590a1ae696b80f4e440c5e21889

commit 07316673b145c590a1ae696b80f4e440c5e21889
Author: Erik Chen <erikchen@chromium.org>
Date: Wed May 09 22:29:53 2018

[Reland 1]: Use unique_ptr in TabDragController and CreateNewStripWithContents.

There was an uninitialized read in test-only code. Not sure why this wasn't
caught by MSAN prior to my original CL. This reland fixes the bug.

> This CL is a refactor with no intended behavior change.

Change-Id: Idae277fa5bad01cb0cf911f9acc3dacdbb7d12b2
Bug: 832879
Reviewed-on: https://chromium-review.googlesource.com/1048593
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557345}
[modify] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/permissions/permission_request_manager_browsertest.cc
[modify] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/ui/browser_tab_strip_model_delegate.cc
[modify] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/ui/browser_tab_strip_model_delegate.h
[modify] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/ui/cocoa/browser_window_controller.mm
[add] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/ui/tabs/tab_strip_model_delegate.cc
[modify] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/ui/tabs/tab_strip_model_delegate.h
[modify] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/ui/tabs/test_tab_strip_model_delegate.cc
[modify] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/ui/tabs/test_tab_strip_model_delegate.h
[modify] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/ui/views/tabs/tab_drag_controller.cc
[modify] https://crrev.com/07316673b145c590a1ae696b80f4e440c5e21889/chrome/browser/ui/views/tabs/tab_drag_controller.h

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d38ec15f7bac87f25a53172e4f21e0d950618ca

commit 3d38ec15f7bac87f25a53172e4f21e0d950618ca
Author: Lucas Furukawa Gadani <lfg@chromium.org>
Date: Fri Oct 26 20:55:18 2018

Use strong ownership for attached inner WebContents.

Bug: 832879

Change-Id: Ib39e8caade4114b396337fa2efc04f4a54f69ed3
Reviewed-on: https://chromium-review.googlesource.com/c/1301843
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603190}
[modify] https://crrev.com/3d38ec15f7bac87f25a53172e4f21e0d950618ca/components/guest_view/browser/guest_view_base.cc
[modify] https://crrev.com/3d38ec15f7bac87f25a53172e4f21e0d950618ca/components/guest_view/browser/guest_view_message_filter.cc
[modify] https://crrev.com/3d38ec15f7bac87f25a53172e4f21e0d950618ca/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/3d38ec15f7bac87f25a53172e4f21e0d950618ca/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/3d38ec15f7bac87f25a53172e4f21e0d950618ca/content/public/browser/web_contents.h
[modify] https://crrev.com/3d38ec15f7bac87f25a53172e4f21e0d950618ca/content/public/test/test_utils.cc

►

Issue 832879 link

Issue metadata

Refactor: Create clean ownership semantics for content::WebContents

Issue description

Comment 1 by bugdroid1@chromium.org, Apr 16 2018

Comment 1 Deleted

Comment 2 by bugdroid1@chromium.org, Apr 17 2018

Comment 2 Deleted

Comment 3 by bugdroid1@chromium.org, Apr 20 2018

Comment 3 Deleted

Comment 4 by bugdroid1@chromium.org, Apr 24 2018

Comment 4 Deleted

Comment 5 by bugdroid1@chromium.org, Apr 24 2018

Comment 5 Deleted

Comment 6 by bugdroid1@chromium.org, Apr 24 2018

Comment 6 Deleted

Comment 7 by bugdroid1@chromium.org, Apr 24 2018

Comment 7 Deleted

Comment 8 by bugdroid1@chromium.org, Apr 26 2018

Comment 8 Deleted

Comment 9 by bugdroid1@chromium.org, Apr 27 2018

Comment 9 Deleted

Comment 10 by bugdroid1@chromium.org, Apr 28 2018

Comment 10 Deleted

Comment 11 by erikc...@chromium.org, Apr 30 2018

Comment 11 Deleted

Comment 12 by bugdroid1@chromium.org, May 1 2018

Comment 12 Deleted

Comment 13 by bugdroid1@chromium.org, May 2 2018

Comment 13 Deleted

Comment 14 by bugdroid1@chromium.org, May 2 2018

Comment 14 Deleted

Comment 15 by bugdroid1@chromium.org, May 3 2018

Comment 15 Deleted

Comment 16 by bugdroid1@chromium.org, May 4 2018

Comment 16 Deleted

Comment 17 by bugdroid1@chromium.org, May 5 2018

Comment 17 Deleted

Comment 18 by bugdroid1@chromium.org, May 5 2018

Comment 18 Deleted

Comment 19 by bugdroid1@chromium.org, May 5 2018

Comment 19 Deleted

Comment 20 by bugdroid1@chromium.org, May 5 2018

Comment 20 Deleted

Comment 21 by bugdroid1@chromium.org, May 5 2018

Comment 21 Deleted

Comment 22 by bugdroid1@chromium.org, May 7 2018

Comment 22 Deleted

Comment 23 by bugdroid1@chromium.org, May 7 2018

Comment 23 Deleted

Comment 24 by bugdroid1@chromium.org, May 9 2018

Comment 24 Deleted

Comment 25 by bugdroid1@chromium.org, Oct 26

Comment 25 Deleted

Sign in to add a comment