Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2258183003:
Make GreasemonkeyApiJsString truly singleton. (Closed)
|
Created:
4 years, 4 months ago by lazyboy Modified:
4 years, 4 months ago Reviewers:
Devlin 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. |
DescriptionMake GreasemonkeyApiJsString truly singleton.
Before this change, ::GetSource() would create a new string from
StringPiece every time.
BUG=622464
Committed: https://crrev.com/de26faaca0525f21a71dfd440d113069b7fb74ba
Cr-Commit-Position: refs/heads/master@{#413478}
Patch Set 1 #
Total comments: 2
Patch Set 2 : address comments + sync #Messages
Total messages: 18 (12 generated)
The CQ bit was checked by lazyboy@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...
lazyboy@chromium.org changed reviewers: + rdevlin.cronin@chromium.org
LGTM https://codereview.chromium.org/2258183003/diff/1/extensions/renderer/user_sc... File extensions/renderer/user_script_injector.cc (right): https://codereview.chromium.org/2258183003/diff/1/extensions/renderer/user_sc... extensions/renderer/user_script_injector.cc:68: // Data is owned by ResourceBundle. Not quite true, right? fromUTF8() still performs a copy of the data and makes a refcounted storage (so this initial copy is the only one), but the data will be owned by WebString storage. So I would just omit this comment entirely.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by lazyboy@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...
https://codereview.chromium.org/2258183003/diff/1/extensions/renderer/user_sc... File extensions/renderer/user_script_injector.cc (right): https://codereview.chromium.org/2258183003/diff/1/extensions/renderer/user_sc... extensions/renderer/user_script_injector.cc:68: // Data is owned by ResourceBundle. On 2016/08/20 01:58:36, Devlin wrote: > Not quite true, right? That's right. Removed the comment. fromUTF8() still performs a copy of the data and makes a > refcounted storage (so this initial copy is the only one), but the data will be > owned by WebString storage. > > So I would just omit this comment entirely.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by lazyboy@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rdevlin.cronin@chromium.org Link to the patchset: https://codereview.chromium.org/2258183003/#ps20001 (title: "address comments + sync")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Make GreasemonkeyApiJsString truly singleton. Before this change, ::GetSource() would create a new string from StringPiece every time. BUG=622464 ========== to ========== Make GreasemonkeyApiJsString truly singleton. Before this change, ::GetSource() would create a new string from StringPiece every time. BUG=622464 Committed: https://crrev.com/de26faaca0525f21a71dfd440d113069b7fb74ba Cr-Commit-Position: refs/heads/master@{#413478} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/de26faaca0525f21a71dfd440d113069b7fb74ba Cr-Commit-Position: refs/heads/master@{#413478} |
