Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(7)

Issue 2103663002: [Extensions] Rework inline installation observation (Closed)

Created:
3 years, 1 month ago by Devlin
Modified:
3 years, 1 month ago
Reviewers:
lazyboy
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Extensions] Rework inline installation observation Instead of observing through the WebstoreAPI, observe directly in the TabHelper. This is a great deal less code, more direct, and also fixes a lifetime issue with the TabHelper being deleted before the inline installation completes. BUG=613949 Committed: https://crrev.com/d30a8bd191f17b61938fc87890bffc80049b0774 Cr-Commit-Position: refs/heads/master@{#403188}

Patch Set 1 : woo #

Total comments: 23

Patch Set 2 : Istiaque's #

Total comments: 2

Patch Set 3 : Nits #

Patch Set 4 : missingfile #

Unified diffs Side-by-side diffs Delta from patch set Stats (+248 lines, -290 lines) Patch
D chrome/browser/extensions/api/webstore/webstore_api.h View 1 chunk +0 lines, -104 lines 0 comments Download
D chrome/browser/extensions/api/webstore/webstore_api.cc View 1 chunk +0 lines, -141 lines 0 comments Download
M chrome/browser/extensions/browser_context_keyed_service_factories.cc View 2 chunks +0 lines, -2 lines 0 comments Download
M chrome/browser/extensions/tab_helper.h View 1 4 chunks +12 lines, -0 lines 0 comments Download
M chrome/browser/extensions/tab_helper.cc View 1 7 chunks +114 lines, -21 lines 0 comments Download
M chrome/browser/extensions/webstore_inline_installer_browsertest.cc View 1 2 chunks +28 lines, -0 lines 0 comments Download
M chrome/browser/extensions/webstore_standalone_installer.cc View 1 11 chunks +14 lines, -20 lines 0 comments Download
M chrome/chrome_browser_extensions.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/chrome_common.gypi View 1 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/extensions/webstore_install_result.h View 1 1 chunk +8 lines, -0 lines 0 comments Download
A chrome/common/extensions/webstore_install_result.cc View 1 2 3 1 chunk +20 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/webstore_inline_install/double_install.html View 1 2 1 chunk +50 lines, -0 lines 0 comments Download

Messages

Total messages: 15 (6 generated)
Devlin
Istiaque, mind taking a look?
3 years, 1 month ago (2016-06-29 00:49:23 UTC) #4
lazyboy
https://codereview.chromium.org/2103663002/diff/40001/chrome/browser/extensions/tab_helper.cc File chrome/browser/extensions/tab_helper.cc (right): https://codereview.chromium.org/2103663002/diff/40001/chrome/browser/extensions/tab_helper.cc#newcode94 chrome/browser/extensions/tab_helper.cc:94: DCHECK(observe_download_progress || observe_install_stage); DCHECK(tab_helper_) https://codereview.chromium.org/2103663002/diff/40001/chrome/browser/extensions/tab_helper.cc#newcode132 chrome/browser/extensions/tab_helper.cc:132: TabHelper* tab_helper_; Mark ...
3 years, 1 month ago (2016-06-29 21:03:04 UTC) #5
Devlin
https://codereview.chromium.org/2103663002/diff/40001/chrome/browser/extensions/tab_helper.cc File chrome/browser/extensions/tab_helper.cc (right): https://codereview.chromium.org/2103663002/diff/40001/chrome/browser/extensions/tab_helper.cc#newcode94 chrome/browser/extensions/tab_helper.cc:94: DCHECK(observe_download_progress || observe_install_stage); On 2016/06/29 21:03:03, lazyboy wrote: > ...
3 years, 1 month ago (2016-06-29 22:32:09 UTC) #6
lazyboy
lgtm https://codereview.chromium.org/2103663002/diff/40001/chrome/browser/extensions/tab_helper.cc File chrome/browser/extensions/tab_helper.cc (right): https://codereview.chromium.org/2103663002/diff/40001/chrome/browser/extensions/tab_helper.cc#newcode132 chrome/browser/extensions/tab_helper.cc:132: TabHelper* tab_helper_; On 2016/06/29 22:32:09, Devlin wrote: > ...
3 years, 1 month ago (2016-06-29 23:10:28 UTC) #7
Devlin
https://codereview.chromium.org/2103663002/diff/40001/chrome/browser/extensions/tab_helper.cc File chrome/browser/extensions/tab_helper.cc (right): https://codereview.chromium.org/2103663002/diff/40001/chrome/browser/extensions/tab_helper.cc#newcode447 chrome/browser/extensions/tab_helper.cc:447: if (pending_inline_installations_.count(webstore_item_id) != 0) { On 2016/06/29 23:10:27, lazyboy ...
3 years, 1 month ago (2016-06-30 14:46:55 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2103663002/100001
3 years, 1 month ago (2016-06-30 14:47:18 UTC) #11
commit-bot: I haz the power
Committed patchset #4 (id:100001)
3 years, 1 month ago (2016-06-30 16:02:48 UTC) #12
commit-bot: I haz the power
CQ bit was unchecked.
3 years, 1 month ago (2016-06-30 16:02:59 UTC) #13
commit-bot: I haz the power
3 years, 1 month ago (2016-06-30 16:04:52 UTC) #15
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/d30a8bd191f17b61938fc87890bffc80049b0774
Cr-Commit-Position: refs/heads/master@{#403188}

Powered by Google App Engine
This is Rietveld 408576698