Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2493603003:
Destroy library before deleting g_font_info in ShutdownSDK() (Closed)
DescriptionDestroy library before deleting g_font_info in ShutdownSDK()
FPDF_DestroyLibrary() will eventually call Release on the default
sysfontinfo. Also, should not try to delete this from the metrics
sysfontinfo, as that is what is done in FPDF_FreeDefault...()
BUG=664072
Committed: https://crrev.com/d7e4f1d73fb613b65fa4ef5694c950a81abd26b8
Cr-Commit-Position: refs/heads/master@{#431319}
Patch Set 1 #Patch Set 2 : Remove bad delete #
Total comments: 2
Patch Set 3 : Cleaner #Patch Set 4 : Avoid copy-paste mistake #Messages
Total messages: 22 (13 generated)
Description was changed from ========== Destroy library before deleting g_font_info in ShutdownSDK() FPDF_DestroyLibrary() will eventually call Release on the default sysfontinfo. Also, should not try to delete this from the metrics sysfontinfo, as that is what is done in FPDF_FreeDefault...() BUG=664072 ========== to ========== Destroy library before deleting g_font_info in ShutdownSDK() FPDF_DestroyLibrary() will eventually call Release on the default sysfontinfo. Also, should not try to delete this from the metrics sysfontinfo, as that is what is done in FPDF_FreeDefault...() BUG=664072 ==========
npm@chromium.org changed reviewers: + dsinclair@chromium.org, thestig@chromium.org, tsepez@chromium.org
PTAL
PTAL
The CQ bit was checked by npm@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/2493603003/diff/20001/pdf/pdfium/pdfium_engin... File pdf/pdfium/pdfium_engine.cc (right): https://codereview.chromium.org/2493603003/diff/20001/pdf/pdfium/pdfium_engin... pdf/pdfium/pdfium_engine.cc:322: Probably cleaner to call FPDF_FreeDefaultSystemFontInfo() in this dtor and have it clean up after itself rather than doing from the outside at 657 https://codereview.chromium.org/2493603003/diff/20001/pdf/pdfium/pdfium_engin... pdf/pdfium/pdfium_engine.cc:325: // Not owned. nit: in which case this comment goes away.
The CQ bit was checked by npm@chromium.org to run a CQ dry run
Good idea, done
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 npm@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...
lgtm
lgtm
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 npm@chromium.org
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.
Description was changed from ========== Destroy library before deleting g_font_info in ShutdownSDK() FPDF_DestroyLibrary() will eventually call Release on the default sysfontinfo. Also, should not try to delete this from the metrics sysfontinfo, as that is what is done in FPDF_FreeDefault...() BUG=664072 ========== to ========== Destroy library before deleting g_font_info in ShutdownSDK() FPDF_DestroyLibrary() will eventually call Release on the default sysfontinfo. Also, should not try to delete this from the metrics sysfontinfo, as that is what is done in FPDF_FreeDefault...() BUG=664072 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Destroy library before deleting g_font_info in ShutdownSDK() FPDF_DestroyLibrary() will eventually call Release on the default sysfontinfo. Also, should not try to delete this from the metrics sysfontinfo, as that is what is done in FPDF_FreeDefault...() BUG=664072 ========== to ========== Destroy library before deleting g_font_info in ShutdownSDK() FPDF_DestroyLibrary() will eventually call Release on the default sysfontinfo. Also, should not try to delete this from the metrics sysfontinfo, as that is what is done in FPDF_FreeDefault...() BUG=664072 Committed: https://crrev.com/d7e4f1d73fb613b65fa4ef5694c950a81abd26b8 Cr-Commit-Position: refs/heads/master@{#431319} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/d7e4f1d73fb613b65fa4ef5694c950a81abd26b8 Cr-Commit-Position: refs/heads/master@{#431319} |
