Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2626073007:
Make drag-and-drop work with OOPIF-WebView. (Closed)
DescriptionMake drag-and-drop work with OOPIF-WebView.
This patch adds support for drag-and-drop to/from/over OOPIF-based WebViews.
BUG=617728
Review-Url: https://codereview.chromium.org/2626073007
Cr-Commit-Position: refs/heads/master@{#444801}
Committed: https://chromium.googlesource.com/chromium/src/+/113278e435c9470189456ad501a63238cec65570
Patch Set 1 #
Total comments: 4
Patch Set 2 : Function rename. #
Messages
Total messages: 29 (23 generated)
The CQ bit was checked by paulmeyer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by paulmeyer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by paulmeyer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== WIP to make drag-and-drop work with OOPIF-webview ========== to ========== Make drag-and-drop work with OOPIF-WebView. This patch adds support for drag-and-drop to/from/over OOPIF-based WebViews. BUG=617728 ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
paulmeyer@chromium.org changed reviewers: + nick@chromium.org
lgtm with a naming change https://codereview.chromium.org/2626073007/diff/40001/content/browser/web_con... File content/browser/web_contents/web_contents_view_child_frame.cc (right): https://codereview.chromium.org/2626073007/diff/40001/content/browser/web_con... content/browser/web_contents/web_contents_view_child_frame.cc:183: } else { Under what circumstances do we need this 'else' case? https://codereview.chromium.org/2626073007/diff/40001/content/browser/web_con... File content/browser/web_contents/web_contents_view_child_frame.h (right): https://codereview.chromium.org/2626073007/diff/40001/content/browser/web_con... content/browser/web_contents/web_contents_view_child_frame.h:73: RenderViewHostDelegateView* GetOuterRVHDV(); It is unconventional to use an abbreviation like RVHDV (which doesn't appear anywhere else in the code) in a method name, unless the abbreviation is extremely well known (like ID, HTML, or URL). This should be spell it out with words (maybe GetOuterDelegateView ? )
paulmeyer@chromium.org changed reviewers: + wjmaclean@chromium.org
+wjmaclean@ for test review. https://codereview.chromium.org/2626073007/diff/40001/content/browser/web_con... File content/browser/web_contents/web_contents_view_child_frame.cc (right): https://codereview.chromium.org/2626073007/diff/40001/content/browser/web_con... content/browser/web_contents/web_contents_view_child_frame.cc:183: } else { On 2017/01/18 19:16:35, ncarter wrote: > Under what circumstances do we need this 'else' case? I'm not sure exactly when this can happen, but this is modeled after the same logic for the BrowserPlugin case: https://cs.chromium.org/chromium/src/content/browser/web_contents/web_content... . https://codereview.chromium.org/2626073007/diff/40001/content/browser/web_con... File content/browser/web_contents/web_contents_view_child_frame.h (right): https://codereview.chromium.org/2626073007/diff/40001/content/browser/web_con... content/browser/web_contents/web_contents_view_child_frame.h:73: RenderViewHostDelegateView* GetOuterRVHDV(); On 2017/01/18 19:16:35, ncarter wrote: > It is unconventional to use an abbreviation like RVHDV (which doesn't appear > anywhere else in the code) in a method name, unless the abbreviation is > extremely well known (like ID, HTML, or URL). This should be spell it out with > words (maybe GetOuterDelegateView ? ) Done.
The CQ bit was checked by paulmeyer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by paulmeyer@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org Link to the patchset: https://codereview.chromium.org/2626073007/#ps60001 (title: "Function rename.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484851252098150,
"parent_rev": "5ee39c9a1b872d95800d7527dbc2ffe7b16cab93", "commit_rev":
"113278e435c9470189456ad501a63238cec65570"}
Message was sent while issue was closed.
Description was changed from ========== Make drag-and-drop work with OOPIF-WebView. This patch adds support for drag-and-drop to/from/over OOPIF-based WebViews. BUG=617728 ========== to ========== Make drag-and-drop work with OOPIF-WebView. This patch adds support for drag-and-drop to/from/over OOPIF-based WebViews. BUG=617728 Review-Url: https://codereview.chromium.org/2626073007 Cr-Commit-Position: refs/heads/master@{#444801} Committed: https://chromium.googlesource.com/chromium/src/+/113278e435c9470189456ad501a6... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:60001) as https://chromium.googlesource.com/chromium/src/+/113278e435c9470189456ad501a6... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
